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>
