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