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