Hooray! Looks like we do have some docs to update now. On Mon, 25 May 2020 at 17:21, Ralph Goers <ralph.go...@dslextreme.com> wrote:
> 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> > >>> > >>> > > > > > -- Matt Sicker <boa...@gmail.com>
