Somebody mind giving me write access to the GitHub repo, please? I am
not able to merge the PR.

On Fri, May 22, 2020 at 3:53 PM Apache <ralph.go...@dslextreme.com> wrote:
>
> Feel free to merge it. I will test it there when I can.
>
> Ralph
>
>
> > On May 22, 2020, at 4:50 AM, Volkan Yazıcı <volkan.yaz...@gmail.com> wrote:
> >
> > Hey Ralph,
> >
> > Here is my status update:
> >
> > -~- Benchmarks -~-
> >
> > I have removed the benchmarks results. It takes ~8h for a complete run
> > and I don't want to repeat that cycle after every change. I might
> > revisit this idea once JTL becomes more stable in terms of its
> > features.
> >
> > -~- Flattening of MDC fields -~-
> >
> > I have changed the MDC directive as follows:
> >
> >    mdc
> >    mdc:flatten[=<prefix>][,stringify]
> >    mdc:pattern=<pattern>[,flatten=<prefix>][,stringify]
> >    mdc:key=<key>[,stringify]
> >
> > This also allowed JsonTemplateLayout to produce the *exact* output as
> > GelfLayout and EcsLayout, where MDC keys are flattened and values are
> > stringified.
> >
> > -~- Null termination, newlines, Logstash, and etc. -~-
> >
> > I have tried to reproduce the experiment you have shared, though could
> > not really succeed due to some Docker command failure in restartApp.sh.
> > Further, I find the associated Spring setup quite cumbersome to wrap my
> > mind around it. That said, I have done something else: I have improved
> > LogstashIT such that
> >
> > 1. Logstash is configured with both "tcp" and "gelf" input plugins,
> >
> > 2. LogstashIT employs JsonTemplateLayout against both inputs, repeats
> >   the same using GelfLayout and EcsLayout, and verifies the populated
> >   content in Elasticsearch.
> >
> > One can easily validate this on the branch as follows:
> >
> >    $ ./mvnw clean package -DskipTests
> >    $ ./mvnw \
> >        verify -o -P docker \
> >        -pl 
> > log4j-layout-jackson-json,log4j-plugins,log4j-core,log4j-api,log4j-layout-json-template
> > \
> >        -Dtest="Dummy.java" -DtrimStackTrace=false -DfailIfNoTests=false
> >
> > One can repeat the very same by running LogstashIT in IDE after
> > starting the containers:
> >
> >    $ ./mvnw \
> >        docker:start -o -P docker \
> >        -pl 
> > log4j-layout-jackson-json,log4j-plugins,log4j-core,log4j-api,log4j-layout-json-template
> >
> > I don't know what is wrong with the Spring Cloud Config setup you have
> > shared, but I have no reasons to believe that JsonTemplateLayout is the
> > suspect.
> >
> > Regarding your remark about everything getting escaped... This might
> > happen when Logstash fails to read an input. In such a case, it puts
> > the entire payload into the "message" field (hence, the escaping) and
> > stores it like that.
> >
> > From now on, I don't know how to proceed with this problem, in
> > particular, given I believe the problem is in somewhere else but
> > JsonTemplateLayout.
> >
> > -~- Merging branch to master -~-
> >
> > Unless there are objections, I want to merge the branch to master.
> > There on I will share json-template-layout.md with the community to
> > collect some feedback on the API. In particular, I have existing users
> > of LogstashLayout in mind.
> >
> > Kind regards.
> >
> >> On Mon, May 18, 2020 at 1:18 AM Ralph Goers <ralph.go...@dslextreme.com> 
> >> wrote:
> >>
> >>
> >>
> >>>> On May 17, 2020, at 2:37 PM, Volkan Yazıcı <volkan.yaz...@gmail.com> 
> >>>> wrote:
> >>>
> >>> Thanks so much for the thorough review Ralph, really appreciated! I
> >>> will address issues you have raised.
> >>>
> >>> [As a side note, I have pushed changes containing performance
> >>> improvements and benchmark results. The module is still dependency
> >>> free and performance-wise pretty good.]
> >>>
> >>>> all the default templates separate the message from the exception
> >>>
> >>> Yes, this I have also realized while studying the source code of
> >>> GelfLayout. Some random thoughts:
> >>>
> >>> 1. How about introducing a firstNonNull-like operator:
> >>>  ${json:op:firstNonNull:${json:message},${json:exception:message}}.
> >>>  Parsing this would not be trivial, given the list of variables
> >>>  can include comma in various forms. Though the rest should
> >>>  be tractable. (Feels like JsonTemplateLayout directives will at
> >>>  some point catch up the with ones in PatternLayout.)
> >>
> >> The problem here is that I want newlines in the message. I’m not sure what 
> >> firstNonNull means in the context of a message. The message itself won’t 
> >> have nulls. The Gelf format dictates the null is at the end of the JSON 
> >> string.
> >>
> >>>
> >>> 2. How about introducing a fallback parameter to the "message"
> >>>  directive: ${json:message:fallback=exception:message}.
> >>
> >> Again, I don’t know what that means or how it solves my problem of wanting 
> >> newlines in the message.
> >>
> >>>
> >>> 3. May I challenge the request: Do one really need it? You store
> >>>  individual values, i.e, message and exception, in individual fields.
> >>>  When one is missing and the other is present, isn't it just a
> >>>  presentation layer issue to display it properly?
> >>
> >> Picture how a log event looks in a file with a “standard” PatternLayout. 
> >> In Kibana by default you get 3 columns, the timestamp, a field I don’t 
> >> care about and the message. I want the message to appear just as it would 
> >> in a log file without the timestamp. Yes, I can click on each message and 
> >> see its attributes, but that takes longer. And I want exceptions to jump 
> >> out at me when I am visually scrolling through messages. In fact, I 
> >> typically don’t want the stack trace in a variable of its own.   Kibana 
> >> doesn’t take the pieces of the message and put them together like the 
> >> pattern layout does. Instead it lets you create columns for individual 
> >> attributes. The problem with that is the columns waste a lot of space, 
> >> especially if they frequently are empty as a stack trace would be.
> >>
> >> So yes, I really need it.
> >>
> >>>
> >>>> In the logged event all newlines seem to be stripped from the exception.
> >>>
> >>> I really don't get this Ralph. Have you seen the tests in LogstashIT?
> >>> I have just added a new one testing only newlines. Would you mind
> >>> fiddling around with LogstashIT to reproduce the issue, please? Or
> >>> sharing a recipe for me to reproduce it?
> >>
> >> The way I do it is to install Logstash, ElasticSearch and Kibana locally 
> >> on my Mac as well as Docker Desktop for Mac. I then run SpringCloud config 
> >> server from the Log4j Spring Cloud Config project and tailor the 
> >> log4j2.xml that is in the config-repo directory of that project. Note that 
> >> all Log4j Lookup variables have to have the $ escaped or Spring Cloud 
> >> Config will try to resolve them. It also requires having the Socket 
> >> appender connect to a host of “host.docker.internal” to be able to connect 
> >> to Logstash outside of the docker container. I configure Logstash using 
> >> the configuration documented at 
> >> http://logging.apache.org/log4j/2.x/manual/cloud.html 
> >> <http://logging.apache.org/log4j/2.x/manual/cloud.html>, at least when 
> >> using Gelf. I configure Kibana with an index of app-*.
> >>
> >> I then modifythe Spring Cloud Config Sample app as needed to work with 
> >> this setup and start it using docker/restartApp.sh.
> >>
> >> After starting I do a few things:
> >> 1. Enter “docker logs app-container”. This should show it is running and 
> >> the last message will be the Spring logo.
> >> 2. Look in Kibana and click on the logs. You should see logs from the 
> >> server.
> >> 3. In a browser enter http://localhost:8080/sample/exception 
> >> <http://localhost:8080/sample/exception> - unless you have changed the 
> >> port. For some reason I had something running on 8080 and had to change 
> >> the sample app to use 8090. This will generate a stack trace on the web 
> >> page and in the log.
> >> 4. Look in Kibana. You should see the exception and the stack trace 
> >> properly formatted in the message column. It should also display the 
> >> thread id, log-level and other items configured in the pattern.
> >>
> >>>
> >>>> Logstash does not seem to be getting a null character
> >>>> (I tried with both “\0” and “\u0000”)
> >>>
> >>> This is weird. Below is how GelfLayout does it:
> >>>
> >>>   if (includeNullDelimiter) {
> >>>       builder.append('\0');
> >>>   }
> >>>
> >>> And here is how JsonTemplateLayout does it:
> >>>
> >>>   stringBuilder.append(eventDelimiter);
> >>>
> >>> Further, there is even a test for this:
> >>> JsonTemplateLayout#test_null_eventDelimiter():
> >>>
> >>>   final String serializedLogEvent = layout.toSerializable(logEvent);
> >>>   assertThat(serializedLogEvent).isEqualTo(eventTemplate + '\0');
> >>>
> >>> Any ideas on what might I be missing?
> >>
> >> No - I just know that Logstash wasn’t detecting the null. It looked like 
> >> the whole string was being escaped as all the double quotes were also 
> >> escaped as you can see from the example. I should note that I directly 
> >> copied that from the Logstash log that was written to the terminal window 
> >> with debug logging enabled, so it is possible it was caused by that, but 
> >> this is also what I see in Kibana.
> >>
> >>>
> >>>> This isn’t valid in the Gelf spec as additional attributes in Gelf
> >>>> must match the regex - ^[\w\.\-]*$. This means you need to
> >>>> parse the MDC and add each key as an additional to be able
> >>>> to create valid Gelf. Personally, I’d like that option anyway.
> >>>
> >>> For this purpose, I need to introduce an operator similar to
> >>> unquote-splicing in Lisp to merge the contents of the child with the
> >>> parent. That said, I have my doubts about the practical benefit of
> >>> such a feature. To the best of my knowledge, almost every (sanely
> >>> configured?) log pipeline ends up with a structured storage system
> >>> containing whitelisted fields. Consider ELK stack. Would you allow
> >>> developers to introduce fields at their will? If so, it is a matter of
> >>> time somebody will take down the entire Elasticsearch cluster by
> >>> flooding it with ContextData.put(userInput, "doh"). Hence, you employ
> >>> the identical whitelisting strategy at the layout as well:
> >>>
> >>>   {
> >>>     "version": "1.1",
> >>>     "host": "${hostName}",
> >>>     ...,
> >>>     "_mdc.loginId": "${json:mdc:loginId}",
> >>>     "_mdc.requestId": "${json:mdc:requestId}"
> >>>   }
> >>>
> >>> What do you think? Shall we really introduce unquote-splicing?
> >>
> >> I would have preferred a syntax like
> >>
> >>   {
> >>     “version” “1.1”,
> >>     …
> >>     ${json.map:mdc:prefix=“_”}
> >>   }
> >>
> >> This would cause the whole map to be emitted. Although it would be nice to 
> >> have a version of that which also could specify the keys there probably 
> >> isn’t a point to that since you can directly specify them as you have 
> >> shown.
> >>
> >>
> >> As I said previously, now that I understand what is going on I am 
> >> comfortable with you merging the PR. It is going to the master branch so 
> >> it won’t be what users see on the web site right now. By the time it makes 
> >> it there we can get this all straightened out.  If nothing else we could 
> >> have the current GelfLayout and the JsonTemplateLayout both on the page 
> >> and explain the differences between them, although in the end I agree it 
> >> would be better to have the JsonTemplateLayout replace both the JsonLayout 
> >> and GelfLayout.
> >>
> >> Ralph
> >>
> >>>
> >>> Kind regards.
> >>>
> >>> On Sat, May 16, 2020 at 1:28 AM Ralph Goers <ralph.go...@dslextreme.com> 
> >>> wrote:
> >>>>
> >>>> I finally got time to spend looking at the Layout. I now understand why 
> >>>> it works for you and sort of understand why it doesn’t work for me.
> >>>>
> >>>> First, all the default templates separate the message from the 
> >>>> exception. In the logged event all newlines seem to be stripped from the 
> >>>> exception. This makes viewing them in Kibana pretty awful, but the do 
> >>>> get logged successfully.
> >>>>
> >>>> To include newlines in a log event you either have to play games in 
> >>>> Logstash (or Fluentd) and try to put the event back together again after 
> >>>> it has been separated into multiple lines or you have to use a different 
> >>>> delimiter. Gelf specifies that a null character be used.
> >>>>
> >>>> I then tried to use GelfLayout.json with
> >>>>
> >>>> <Socket name="Logstash"
> >>>>       host="\${sys:logstash.host:-host.docker.internal}"
> >>>>       port="12222"
> >>>>       protocol="tcp"
> >>>>       bufferedIo="true">
> >>>> <JsonTemplateLayout eventTemplateUri="classpath:GelfLayout.json" 
> >>>> eventDelimiter="\0"
> >>>>     locationInfoEnabled="true">
> >>>>   <EventTemplateAdditionalFields>
> >>>>     <KeyValuePair key="hostName" value="${hostName}"/>
> >>>>     <KeyValuePair key="_containerId" value="\${docker:containerId:-}"/>
> >>>>     <KeyValuePair key="_application" 
> >>>> value="$\${lower:\${spring:spring.application.name:-spring}}"/>
> >>>>   </EventTemplateAdditionalFields>
> >>>> </JsonTemplateLayout>
> >>>> </Socket>
> >>>> but this did not work at all. Logstash does not seem to be getting a 
> >>>> null character (I tried with both “\0” and “\u0000”) and all the events 
> >>>> buffer in Logstash until I shutdown the application. Worse the 
> >>>> formatting looks like escaped JSON rather than raw json.
> >>>>
> >>>> Also, the GelfTemplate contains
> >>>> {
> >>>> "version": "1.1",
> >>>> "host": "${hostName}",
> >>>> "short_message": "${json:message}",
> >>>> "full_message": "${json:exception:stackTrace:text}",
> >>>> "timestamp": "${json:timestamp:epoch:secs}",
> >>>> "level": "${json:level:severity:code}",
> >>>> "_logger": "${json:logger:name}",
> >>>> "_thread": "${json:thread:name}",
> >>>> "_mdc": "${json:mdc}"
> >>>> }
> >>>> This causes the output to contain
> >>>>
> >>>> \\\"_mdc\\\":{\\\"loginId\\\":\\\"rgoers\\\",\\\"requestId\\\":\\\"266196c1-9702-11ea-9e4a-0242ac120006\\\"}
> >>>>
> >>>> This isn’t valid in the Gelf spec as additional attributes in Gelf must 
> >>>> match the regex - ^[\w\.\-]*$. This means you need to parse the MDC and 
> >>>> add each key as an additional to be able to create valid Gelf. 
> >>>> Personally, I’d like that option anyway.
> >>>>
> >>>> I would suggest you attempt to work to make a template that matches the 
> >>>> result of the GelfLayout I had previously documented.
> >>>>
> >>>> But now that I understand what is going on I am OK with this 
> >>>> contribution being merged. I would like to get these issues addressed 
> >>>> and update the Logging in the Cloud documentation so that it can 
> >>>> document both ways of sending to Logstash but we can do that after the 
> >>>> code is merged.
> >>>>
> >>>> Ralph
> >>>>
> >>>>
> >>>>
> >>>>
> >>>>
> >>>>> On Apr 21, 2020, at 2:23 PM, Ralph Goers <ralph.go...@dslextreme.com> 
> >>>>> wrote:
> >>>>>
> >>>>> Ok. Now that a release has been cut spending time with your 
> >>>>> contribution is at the top of my list.
> >>>>>
> >>>>> Ralph
> >>>>>
> >>>>>> On Apr 21, 2020, at 1:24 PM, Volkan Yazıcı <volkan.yaz...@gmail.com> 
> >>>>>> wrote:
> >>>>>>
> >>>>>> Ralph, I've just pushed a commit that adds PatternLayout support, FYI.
> >>>>>>
> >>>>>> On Mon, Apr 20, 2020 at 2:08 AM Ralph Goers 
> >>>>>> <ralph.go...@dslextreme.com> wrote:
> >>>>>>>
> >>>>>>> I completely understand how messed up people’s lives are right now. 
> >>>>>>> I’m lucky, all my kids are long gone out of the house. With COVID-19 
> >>>>>>> I am able to work from home and while that saves me over 2 hours a 
> >>>>>>> day in commute time and more time because there isn’t much else to 
> >>>>>>> do, I end up spending a lot of the extra time on my $dayjob (and feel 
> >>>>>>> lucky I still have it).
> >>>>>>>
> >>>>>>> 1. You should care about the checkstyle errors as much as everyone 
> >>>>>>> else ;-). They don’t seem to be a high priority for people to fix.
> >>>>>>>
> >>>>>>> 2. Maybe I didn’t make my need for the Pattern Layout clear. Adding 
> >>>>>>> the pattern layout for the JTL is ONLY for the message attribute 
> >>>>>>> (just as I did for the GelfLayout). This allows what is displayed in 
> >>>>>>> Kibana to look like what you would see in the log file while still 
> >>>>>>> allowing filtering on all the attributes. For me, this is a big deal 
> >>>>>>> - I won’t use the Layout without it. That doesn’t mean it has to be 
> >>>>>>> included in the initial commit.  I don’t believe JsonLayout supports 
> >>>>>>> it either but I am hoping to be able to do that today, although I 
> >>>>>>> have several other things I need to do. What you would end up with is 
> >>>>>>> a new attribute called messagePattern that takes a pattern that is 
> >>>>>>> exactly what you would give to PatternLayout.
> >>>>>>>
> >>>>>>> 3. You can handle the performance issue however you want but if it 
> >>>>>>> was me I would just revert back to using Jackson without a second 
> >>>>>>> thought. I just don’t see the point of trying to roll your own.
> >>>>>>>
> >>>>>>> 4. It will take me a while to see what your integration test does and 
> >>>>>>> if it resolves my concerns regarding updating the logging in the 
> >>>>>>> cloud page.
> >>>>>>>
> >>>>>>> Ralph
> >>>>>>>
> >>>>>>>
> >>>>>>>
> >>>>>>>
> >>>>>>>> On Apr 19, 2020, at 2:30 PM, Volkan Yazıcı <volkan.yaz...@gmail.com> 
> >>>>>>>> wrote:
> >>>>>>>>
> >>>>>>>> Hello Ralph,
> >>>>>>>>
> >>>>>>>> [Due to #COVID-19, day cares are closed. I can hardly spare time to
> >>>>>>>> work, including Log4j. Hence, apologies for my slowed down progress &
> >>>>>>>> responses.]
> >>>>>>>>
> >>>>>>>> I could not put my finger on the performance regression, yet. I might
> >>>>>>>> opt to switch back to Jackson, but I am not gonna do that without
> >>>>>>>> spotting that darn pain in the back. (You might have noticed, there 
> >>>>>>>> is
> >>>>>>>> actually a class for generating a report out of the available
> >>>>>>>> JSON-emitting layouts. I will skip incorporating its output into the
> >>>>>>>> manually until I address this performance regression.)
> >>>>>>>>
> >>>>>>>> I've just pushed a commit, i.e., LogstashIT, which tests
> >>>>>>>> JsonTemplateLayout against ELK stack. Would you mind skimming through
> >>>>>>>> that (small) change, please?
> >>>>>>>>
> >>>>>>>> I am not inclined to incorporate PatternLayout (PL) into
> >>>>>>>> JsonTemplateLayout (JTL). JTL is supposed to (ideally) deliver all 
> >>>>>>>> the
> >>>>>>>> functionality provided by PL but in a structured format, i.e., JSON.
> >>>>>>>> If there arises a need to fallback to PL, I'd rather add that missing
> >>>>>>>> functionality to JTL.
> >>>>>>>>
> >>>>>>>> You've mentioned about the checkstyle errors. How much shall I be
> >>>>>>>> concerned about them? (It even complains about missing JavaDocs on
> >>>>>>>> class fields.) Are there any particular ones you want me to address?
> >>>>>>>>
> >>>>>>>> Besides these... I think I am done.
> >>>>>>>>
> >>>>>>>> Kind regards.
> >>>>>>>>
> >>>>>>>> On Thu, Apr 16, 2020 at 10:49 PM Ralph Goers 
> >>>>>>>> <ralph.go...@dslextreme.com> wrote:
> >>>>>>>>>
> >>>>>>>>> Volkan,
> >>>>>>>>>
> >>>>>>>>> I’m just wondering if you have had a chance to get to this. As I 
> >>>>>>>>> said, as far as I am concerned this is the only thing getting in 
> >>>>>>>>> the way of merging the PR. If you would prefer you can just remove 
> >>>>>>>>> the update to the “Logging in the Cloud” page and push the rest and 
> >>>>>>>>> then update that page after you have the testing done.
> >>>>>>>>>
> >>>>>>>>> Ralph
> >>>>>>>>>
> >>>>>>>>>> On Apr 12, 2020, at 12:01 AM, Ralph Goers 
> >>>>>>>>>> <ralph.go...@dslextreme.com> wrote:
> >>>>>>>>>>
> >>>>>>>>>> I think I said this before, but I see no reason you can’t have a 
> >>>>>>>>>> dependency on Jackson. We currently do for almost all of our JSON 
> >>>>>>>>>> processing. I wouldn’t spend any more time trying to improve the 
> >>>>>>>>>> performance of your custom support, especially since - as I recall 
> >>>>>>>>>> - you had to drop support of a feature because it was hard to 
> >>>>>>>>>> implement.
> >>>>>>>>>>
> >>>>>>>>>> As for replacing JsonLayout and GelfLayout, I would envision that 
> >>>>>>>>>> those would continue to exist for backwards compatibility but 
> >>>>>>>>>> would simply pass their configuration too JsonTemplateLayout.
> >>>>>>>>>>
> >>>>>>>>>> As for moving JSON out of core, we may end up doing that. I am 
> >>>>>>>>>> pretty sure the xml support will require a dependency on the 
> >>>>>>>>>> java.xml module so either that will move out of core or we will 
> >>>>>>>>>> have a transitive dependency on it. Whichever way we go with that 
> >>>>>>>>>> I would expect would be the same with Json and Yaml.
> >>>>>>>>>>
> >>>>>>>>>> Adding support for formatting the message with the PatternLayout 
> >>>>>>>>>> was pretty trivial. I would prefer to continue to have that in the 
> >>>>>>>>>> example so I would hope it could be added if the example changes.
> >>>>>>>>>>
> >>>>>>>>>> As I said, the issue is Logstash or Fluentd and how they process 
> >>>>>>>>>> messages. Adding the null terminator in the configuration would 
> >>>>>>>>>> make it work on the log4j side as long as you left the logstash 
> >>>>>>>>>> configuration alone.  My devops team has tried everything they can 
> >>>>>>>>>> think of to get exceptions to work and this was the only thing 
> >>>>>>>>>> that did without having to invent convoluted parsing rules. But 
> >>>>>>>>>> yet, before we publish that I would need to know it had been 
> >>>>>>>>>> tested all the way to Kibana. It was pretty easy for me to set it 
> >>>>>>>>>> all up in docker containers. I think the log4j-spring-cloud-sample 
> >>>>>>>>>> project has reference to the docker setup I used.
> >>>>>>>>>>
> >>>>>>>>>> Ralph
> >>>>>>>>>>
> >>>>>>>>>>
> >>>>>>>>>>
> >>>>>>>>>>> On Apr 11, 2020, at 11:23 PM, Volkan Yazıcı 
> >>>>>>>>>>> <volkan.yaz...@gmail.com> wrote:
> >>>>>>>>>>>
> >>>>>>>>>>> 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