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 > >>>>>>>>>>> > >>>>>>>>>> > >>>>>>>>>> > >>>>>>>>>> > >>>>>>>>> > >>>>>>>>> > >>>>>>>> > >>>>>>> > >>>>>>> > >>>>>> > >>>>> > >>>>> > >>>>> > >>>> > >>> > >> > > > >