Ralph, I can indeed see I am a member of the Apache organization.
Though I don't see a "merge" option appearing in the PR page. Could it
be because of the "change request" of yours? If so, would you mind
resolving that request, please? (Sorry, I don't see any other visual
cue to proceed further. Maybe it is just me...)

On Mon, May 25, 2020 at 12:02 AM Ralph Goers <ralph.go...@dslextreme.com> wrote:
>
> You should already have it.
>
> Ralph
>
> > On May 24, 2020, at 2:59 PM, Volkan Yazıcı <volkan.yaz...@gmail.com> wrote:
> >
> > 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