Wouldn’t that be � in XML?
On Sat, Jul 11, 2020 at 16:26 Ralph Goers <ralph.go...@dslextreme.com>
wrote:
> I guess what you mean is that where you have $resolver: “message” that
> would somehow be replaced by $resolver” “pattern”? That would make sense.
> I have no problem with the changes you are suggesting. However, you should
> also change the configuration shown in cloud.md and the log4j2.xml in
> log4j-spring-cloud-config/log4j-spring-cloud-samples/log4j-spring-cloud-config-sample-server/src/main/config-repo
> to match.
>
> Thanks for determining that it was XML parsing that was messing with the
> “\0” value. That implies there would have been an XMLish way to bypass that
> but using an explicit attribute is just a lot easier. The GelfLayout uses
> “includeNullDelimitor”. You might consider using that name for consistency.
>
> Ralph
>
> > On Jul 11, 2020, at 1:46 PM, Volkan Yazıcı <volkan.yaz...@gmail.com>
> wrote:
> >
> > I can't believe how were we expecting somebody to pass a null
> > character from an XML file! Long story short, it was the XML parser,
> > doing the right thing, that is, converting XML attribute value "\0" to
> > Java String "\\0". Okay, I am not gonna whine about this anymore. I
> > have just pushed a commit adding `nullEventDelimiterEnabled` flag
> > along with a unit test. (Ralph, I've replaced your
> > `eventDelimiter="null"` handling as well.)
> >
> > Note that my question about reverting the changes to `MessageResolver`
> > still stands.
> >
> > On Sat, Jul 11, 2020 at 8:37 PM Volkan Yazıcı <volkan.yaz...@gmail.com>
> wrote:
> >>
> >> Hello Ralph!
> >>
> >> Sorry that it took me this long to reply to you back in detail, but I
> >> have just found time to do so. Let me try to address the points you
> >> raised:
> >>
> >>> The first problem I ran into is that additional fields don’t work.
> >>> I don’t see any configurations that have them and the Builder doesn’t
> >>> have annotations on the key and value fields so I suspect it just
> >>> doesn’t work.
> >>
> >> Yes, the annotations... :facepalm: Good catch.
> >>
> >>> Why didn’t you just enhance KeyValuePair to add the type attribute?
> >>
> >> I was a little bit hesitant to do that, since `type` is mostly a
> >> concern in the context of JTL. Would you rather recommend me to extend
> >> `KeyValuePair` instead?
> >>
> >>> I thought you said you added the ability to format the message using
> >>> a pattern, but I don’t see that in the documentation or in
> >>> MessageResolver. Was that not included?
> >>
> >> Dang it! Indeed I forgot to add `PatternResolver` into the docs. (I
> >> will talk about this again in the following lines.)
> >>
> >>> I have added the ability to format the message field using a pattern.
> >>
> >> Doh! I totally misinterpreted your request. I thought you wanted to be
> >> able to *only* employ `PatternLayout` in JTL, hence I created
> >> `PatternResolver`. Though what you asked for is to treat the message
> >> as a pattern. (Looking back right now, your request was pretty clear,
> >> it was me who was confused.)
> >>
> >> I have reviewed the changes and I want to share two remarks:
> >>
> >> 1. I've seen that you have used a thread-local. That functionality is
> >> totally delegated to the `RecyclerFactory` employed, which can be
> >> provided by the user. (I will change this accordingly.)
> >>
> >> 2. I've noticed `PatternResolver` is missing the `includeStacktrace`
> >> flag. I will add it.
> >>
> >>> I have modified the code so that specifying `eventDelimiter=“null”`
> >>> will insert a null at the end of the message as specifying
> >>> eventDelimiter=“\0” or “\u000” does not work (It ends up being “\\0”
> >>> or “\u000”.
> >>
> >> I am really puzzled about this. Let me explain it through the sources.
> These
> >> are the lines you have added:
> >>
> >> if (eventDelimiter != null &&
> eventDelimiter.equalsIgnoreCase("null")) {
> >> stringBuilder.append('\0');
> >> } else {
> >> stringBuilder.append(eventDelimiter);
> >> }
> >>
> >> Nowhere in the code `eventDelimiter` is JSON quoted. I am totally
> >> clueless how setting `eventDelimiter` to `\0` doesn't create the
> >> effect you manually execute here. Further, if it doesn't, how come
> >> `JsonTemplateLayoutTest#test_null_eventDelimiter()` test passes? I see
> >> nothing else but to blame two other suspects:
> >>
> >> 1. Either setting the `eventDelimiter` property to `\0` causes the
> >> value to get quoted.
> >>
> >> 2. Or there is something else going on while passing the value from
> >> the layout to the appender.
> >>
> >> On the other hand, I am a little bit distant to creating special cases
> >> for certain values (e.g., `null`) of `eventDelimiter`. What if the
> >> user literally wants to dump `null` (literally!) after every event?
> >>
> >>> As I was implementing stuff I noticed you implemented stuff using
> >>> lots of static methods. I am not really sure why since almost all of
> >>> those methods are only invoked from an instance of the object.
> >>
> >> I generally choose the strictest modifier combination, unless there is
> >> a reason not to do so. Following this principle, if a method/field
> >> doesn't have any instance dependencies, I make it static. Am I doing
> >> something wrong with this convention?
> >>
> >>> Also, I would like to be able to specify the pattern I use for the
> >>> message in the log4j configuration so it is easier to change
> >>> dynamically but I couldn’t figure out how to do that and get the
> >>> pattern passed to the MessageResolver.
> >>
> >> Doesn't `PatternResolver` fit the bill here?
> >>
> >> Cheers!
> >>
> >> On Wed, Jul 8, 2020 at 7:42 PM Ralph Goers <ralph.go...@dslextreme.com>
> wrote:
> >>>
> >>> A couple other things. As I was implementing stuff I noticed you
> implemented stuff using lots of static methods. I am not really sure why
> since almost all of those methods are only invoked from an instance of the
> object. I had to change that to get my enhancements to work.
> >>>
> >>> Also, I would like to be able to specify the pattern I use for the
> message in the log4j configuration so it is easier to change dynamically
> but I couldn’t figure out how to do that and get the pattern passed to the
> MessageResolver.
> >>>
> >>> Ralph
> >>>
> >>>> On Jul 8, 2020, at 7:16 AM, Volkan Yazıcı <volkan.yaz...@gmail.com>
> wrote:
> >>>>
> >>>> Hey Ralph,
> >>>>
> >>>> Thanks so much for taking time, really appreciated. Unfortunately the
> >>>> last few days I was pretty busy both at home and at work. I will check
> >>>> out your changes and reply back as soon as I find some time.
> >>>>
> >>>> Kind regards.
> >>>>
> >>>> On Wed, Jul 8, 2020 at 1:08 AM Ralph Goers <
> ralph.go...@dslextreme.com> wrote:
> >>>>>
> >>>>> Volkan,
> >>>>>
> >>>>> I have updated the relevant documentation. I am done with all the
> changes I wanted to make.
> >>>>>
> >>>>> Ralph
> >>>>>
> >>>>>> On Jul 6, 2020, at 2:50 PM, Ralph Goers <ralph.go...@dslextreme.com>
> wrote:
> >>>>>>
> >>>>>> I have fixed the issue with the annotations. I have added the
> ability to format the message field using a pattern and I have modified the
> code so that specifying eventDelimiter=“null” will insert a null at the end
> of the message as specifying eventDelimiter=“\0” or “\u000” does not work
> (It ends up being “\\0” or “\u000”.
> >>>>>>
> >>>>>> I have verified that with these changes the message is formatted
> properly with Gelf. I am still testing though to see how the ThreadContext
> is being handled.
> >>>>>>
> >>>>>> Ralph
> >>>>>>
> >>>>>>> On Jul 5, 2020, at 11:01 PM, Ralph Goers <
> ralph.go...@dslextreme.com> wrote:
> >>>>>>>
> >>>>>>> I finally got some time to start testing JsonTemplateLayout
> against the logging in the cloud documentation. The first problem I ran
> into is that additional fields don’t work. I don’t see any configurations
> that have them and the Builder doesn’t have annotations on the key and
> value fields so I suspect it just doesn’t work. Why didn’t you just
> enhance KeyValuePair to add the type attribute?
> >>>>>>>
> >>>>>>> After I corrected the EventTemplateAdditionalField plugin I still
> can’t get stack traces the way I want them. I thought you said you added
> the ability to format the message using a pattern, but I don’t see that in
> the documentation or in MessageResolver. Was that not included? As I said,
> I require the stack trace to print in the message field in Kibana, not just
> be a key in the additional data.
> >>>>>>>
> >>>>>>>
> >>>>>>> Ralph
> >>>>>>>
> >>>>>>
> >>>>>>
> >>>>>>
> >>>>>
> >>>>>
> >>>>
> >>>
> >>>
> >
>
>
> --
Matt Sicker <boa...@gmail.com>
