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