Congrats! Just a reminder - you now have write access to all the logging repositories at GitHub. You are welcome to participate in all of them. There are a few that could use more participation.
Ralph > On May 25, 2020, at 1:45 PM, Volkan Yazıcı <volkan.yaz...@gmail.com> wrote: > > BAM! JsonTemplateLayout is merged to master! 🎉🍾 (I needed to wait > some for the GitHub write access to get sync'ed with Apache LDAP.) > > On Mon, May 25, 2020 at 9:46 PM Volkan Yazıcı <volkan.yaz...@gmail.com> wrote: >> >> I had missed the GitBox Account Linking Matt has mentioned. I have >> just completed it and get acknowledged that I "will" have right access >> to the logging-* repositories. I don't know if it is a timing issue, >> but I still cannot see any "merge" buttons available. I will keep on >> refreshing the page. If there is anything else I need to do, please do >> let me know. >> >> On Mon, May 25, 2020 at 6:40 PM Ralph Goers <ralph.go...@dslextreme.com> >> wrote: >>> >>> I see the squash and merge button at the bottom. I just approved the PR so >>> maybe you can see it now? If not, then the steps Matt has provided may be >>> needed. We will need to update the new committer email to reflect this if >>> that is required. >>> >>> If you are still having problems and need me to do it I can merge it, but >>> it would be great to verify that you can do it. >>> >>> Ralph >>> >>>> On May 25, 2020, at 8:49 AM, Matt Sicker <boa...@gmail.com> wrote: >>>> >>>> Make sure your account is linked properly: >>>> https://gitbox.apache.org/setup/ >>>> >>>> On Mon, May 25, 2020 at 07:23 Volkan Yazıcı <volkan.yaz...@gmail.com> >>>> wrote: >>>> >>>>> 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 >>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>> >>>>>>>>>>>>>>> >>>>>>>>>>>>>>> >>>>>>>>>>>>>> >>>>>>>>>>>>> >>>>>>>>>>>>> >>>>>>>>>>>>> >>>>>>>>>>>> >>>>>>>>>>> >>>>>>>>>> >>>>>>>>> >>>>>>>> >>>>>>>> >>>>>>> >>>>>> >>>>>> >>>>> >>>> -- >>>> Matt Sicker <boa...@gmail.com> >>> >>> >
