You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@logging.apache.org by Ralph Goers <ra...@dslextreme.com> on 2020/07/06 06:01:34 UTC
Testing Json Template Layout
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
Re: Testing Json Template Layout
Posted by Volkan Yazıcı <vo...@gmail.com>.
Reading your changes again, I think your `MessageResolver`
improvements are a clone of `PatternResolver`. Hence, I will revert
the `MessageResolver` changes. Objections?
(Sorry that it took awhile for me to get it clicked. Saturday evening...)
On Sat, Jul 11, 2020 at 8:37 PM Volkan Yazıcı <vo...@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 <ra...@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ı <vo...@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 <ra...@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 <ra...@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 <ra...@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
> > >>>>
> > >>>
> > >>>
> > >>>
> > >>
> > >>
> > >
> >
> >
Re: String parameter with escapes (Was: Testing Json Template Layout)
Posted by Ralph Goers <ra...@dslextreme.com>.
We can’t “backport” the code from Java 13 as that is licensed under the GPL.
What I was suggesting (without saying it) was that maybe we don’t need the eventDelimiter parameter. Yes, this should be solved across the board so everything is consistent.
Ralph
> On Jul 15, 2020, at 12:53 AM, Volkan Yazıcı <vo...@gmail.com> wrote:
>
> First let me state that this "how to pass escape characters to plugin
> parameters of type string from different configuration sources"
> problem applies to every other plugin, e.g.,
> `AbstractJacksonLayout.Builder#endOfLine`. Hence, nothing specific to
> `JsonTemplateLayout`.
>
> That said, I share your hesitation to include a relatively complex
> `unescape()` routine into the code base. Though having both
> `eventDelimiter` and `nullEventDelimiterEnabled` parameters while the
> same effect can exactly be created with just `eventDelimiter` feels
> like a code smell to me. What about just backporting
> `translateEscapes()` of Java 13? This will still fit the bill and can
> be removed in the future.
>
> On Tue, Jul 14, 2020 at 7:41 PM Ralph Goers <ra...@dslextreme.com> wrote:
>>
>> Do we really need to do this? The only two delimiters that have ever been requested are null and newline. Defaulting to newline and having an option to use null seems fine to me.
>>
>> Ralph
>>
>>> On Jul 14, 2020, at 5:49 AM, Volkan Yazıcı <vo...@gmail.com> wrote:
>>>
>>> Thinking more on this issue made me question the ideal way to pass
>>> Java string literals to plugins.
>>>
>>> I first gave Matt's suggestion a try, set eventDelimiter="�" in the
>>> XML and got the following error:
>>>
>>> ERROR StatusLogger Error parsing /home/vy/...Logging.xml
>>> org.xml.sax.SAXParseException; systemId:
>>> file:///home/vy/...Logging.xml; lineNumber: 29; columnNumber: 55;
>>> Character reference "&#
>>> at com.sun.org.apache.xerces.internal.parsers.DOMParser.parse(DOMParser.java:257)
>>>
>>> Next I tried passing escapes from a properties files instead. Setting
>>> eventDelimiter=\r\n\0 results in string "\r\n0".
>>>
>>> Hence, I am pretty confident that the user input needs to be escaped
>>> and I need to unescape it upon reception. There is an ad-hoc
>>> `unescapeJava()` method shared in a StackOverflow post[1]. The thread
>>> also references StringEscapeUtils.unescapeJava() (from commons) and
>>> String#translateEscapes() (Java 13+) alternatives along with their
>>> caveats.
>>>
>>> At this stage, I am inclined to pick the solution shared in
>>> StackOverflow[1]. Note that accepting escape characters will void the
>>> need to nullEventDelimiterEnabled flag. To sum it up,
>>>
>>> 1. May I have your blessing for using the StackOverflow snippet?
>>> 2. Is it okay with regards to licensing?
>>>
>>> [1] https://stackoverflow.com/a/4298836/1278899
>>>
>>> On Sun, Jul 12, 2020 at 7:35 PM Ralph Goers <ra...@dslextreme.com> wrote:
>>>>
>>>> Maybe. But it isn’t obvious and would have to be explicitly documented.
>>>>
>>>> Ralph
>>>>
>>>>> On Jul 12, 2020, at 8:09 AM, Matt Sicker <bo...@gmail.com> wrote:
>>>>>
>>>>> Wouldn’t that be � in XML?
>>>>>
>>>>> On Sat, Jul 11, 2020 at 16:26 Ralph Goers <ra...@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ı <vo...@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ı <vo...@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 <ra...@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ı <vo...@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.goers@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 <ra...@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.goers@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 <bo...@gmail.com>
>>>>
>>>>
>>>
>>
>>
>
Re: String parameter with escapes (Was: Testing Json Template Layout)
Posted by Volkan Yazıcı <vo...@gmail.com>.
First let me state that this "how to pass escape characters to plugin
parameters of type string from different configuration sources"
problem applies to every other plugin, e.g.,
`AbstractJacksonLayout.Builder#endOfLine`. Hence, nothing specific to
`JsonTemplateLayout`.
That said, I share your hesitation to include a relatively complex
`unescape()` routine into the code base. Though having both
`eventDelimiter` and `nullEventDelimiterEnabled` parameters while the
same effect can exactly be created with just `eventDelimiter` feels
like a code smell to me. What about just backporting
`translateEscapes()` of Java 13? This will still fit the bill and can
be removed in the future.
On Tue, Jul 14, 2020 at 7:41 PM Ralph Goers <ra...@dslextreme.com> wrote:
>
> Do we really need to do this? The only two delimiters that have ever been requested are null and newline. Defaulting to newline and having an option to use null seems fine to me.
>
> Ralph
>
> > On Jul 14, 2020, at 5:49 AM, Volkan Yazıcı <vo...@gmail.com> wrote:
> >
> > Thinking more on this issue made me question the ideal way to pass
> > Java string literals to plugins.
> >
> > I first gave Matt's suggestion a try, set eventDelimiter="�" in the
> > XML and got the following error:
> >
> > ERROR StatusLogger Error parsing /home/vy/...Logging.xml
> > org.xml.sax.SAXParseException; systemId:
> > file:///home/vy/...Logging.xml; lineNumber: 29; columnNumber: 55;
> > Character reference "&#
> > at com.sun.org.apache.xerces.internal.parsers.DOMParser.parse(DOMParser.java:257)
> >
> > Next I tried passing escapes from a properties files instead. Setting
> > eventDelimiter=\r\n\0 results in string "\r\n0".
> >
> > Hence, I am pretty confident that the user input needs to be escaped
> > and I need to unescape it upon reception. There is an ad-hoc
> > `unescapeJava()` method shared in a StackOverflow post[1]. The thread
> > also references StringEscapeUtils.unescapeJava() (from commons) and
> > String#translateEscapes() (Java 13+) alternatives along with their
> > caveats.
> >
> > At this stage, I am inclined to pick the solution shared in
> > StackOverflow[1]. Note that accepting escape characters will void the
> > need to nullEventDelimiterEnabled flag. To sum it up,
> >
> > 1. May I have your blessing for using the StackOverflow snippet?
> > 2. Is it okay with regards to licensing?
> >
> > [1] https://stackoverflow.com/a/4298836/1278899
> >
> > On Sun, Jul 12, 2020 at 7:35 PM Ralph Goers <ra...@dslextreme.com> wrote:
> >>
> >> Maybe. But it isn’t obvious and would have to be explicitly documented.
> >>
> >> Ralph
> >>
> >>> On Jul 12, 2020, at 8:09 AM, Matt Sicker <bo...@gmail.com> wrote:
> >>>
> >>> Wouldn’t that be � in XML?
> >>>
> >>> On Sat, Jul 11, 2020 at 16:26 Ralph Goers <ra...@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ı <vo...@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ı <vo...@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 <ra...@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ı <vo...@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.goers@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 <ra...@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.goers@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 <bo...@gmail.com>
> >>
> >>
> >
>
>
Re: String parameter with escapes (Was: Testing Json Template Layout)
Posted by Ralph Goers <ra...@dslextreme.com>.
Do we really need to do this? The only two delimiters that have ever been requested are null and newline. Defaulting to newline and having an option to use null seems fine to me.
Ralph
> On Jul 14, 2020, at 5:49 AM, Volkan Yazıcı <vo...@gmail.com> wrote:
>
> Thinking more on this issue made me question the ideal way to pass
> Java string literals to plugins.
>
> I first gave Matt's suggestion a try, set eventDelimiter="�" in the
> XML and got the following error:
>
> ERROR StatusLogger Error parsing /home/vy/...Logging.xml
> org.xml.sax.SAXParseException; systemId:
> file:///home/vy/...Logging.xml; lineNumber: 29; columnNumber: 55;
> Character reference "&#
> at com.sun.org.apache.xerces.internal.parsers.DOMParser.parse(DOMParser.java:257)
>
> Next I tried passing escapes from a properties files instead. Setting
> eventDelimiter=\r\n\0 results in string "\r\n0".
>
> Hence, I am pretty confident that the user input needs to be escaped
> and I need to unescape it upon reception. There is an ad-hoc
> `unescapeJava()` method shared in a StackOverflow post[1]. The thread
> also references StringEscapeUtils.unescapeJava() (from commons) and
> String#translateEscapes() (Java 13+) alternatives along with their
> caveats.
>
> At this stage, I am inclined to pick the solution shared in
> StackOverflow[1]. Note that accepting escape characters will void the
> need to nullEventDelimiterEnabled flag. To sum it up,
>
> 1. May I have your blessing for using the StackOverflow snippet?
> 2. Is it okay with regards to licensing?
>
> [1] https://stackoverflow.com/a/4298836/1278899
>
> On Sun, Jul 12, 2020 at 7:35 PM Ralph Goers <ra...@dslextreme.com> wrote:
>>
>> Maybe. But it isn’t obvious and would have to be explicitly documented.
>>
>> Ralph
>>
>>> On Jul 12, 2020, at 8:09 AM, Matt Sicker <bo...@gmail.com> wrote:
>>>
>>> Wouldn’t that be � in XML?
>>>
>>> On Sat, Jul 11, 2020 at 16:26 Ralph Goers <ra...@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ı <vo...@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ı <vo...@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 <ra...@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ı <vo...@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.goers@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 <ra...@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.goers@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 <bo...@gmail.com>
>>
>>
>
String parameter with escapes (Was: Testing Json Template Layout)
Posted by Volkan Yazıcı <vo...@gmail.com>.
Thinking more on this issue made me question the ideal way to pass
Java string literals to plugins.
I first gave Matt's suggestion a try, set eventDelimiter="�" in the
XML and got the following error:
ERROR StatusLogger Error parsing /home/vy/...Logging.xml
org.xml.sax.SAXParseException; systemId:
file:///home/vy/...Logging.xml; lineNumber: 29; columnNumber: 55;
Character reference "&#
at com.sun.org.apache.xerces.internal.parsers.DOMParser.parse(DOMParser.java:257)
Next I tried passing escapes from a properties files instead. Setting
eventDelimiter=\r\n\0 results in string "\r\n0".
Hence, I am pretty confident that the user input needs to be escaped
and I need to unescape it upon reception. There is an ad-hoc
`unescapeJava()` method shared in a StackOverflow post[1]. The thread
also references StringEscapeUtils.unescapeJava() (from commons) and
String#translateEscapes() (Java 13+) alternatives along with their
caveats.
At this stage, I am inclined to pick the solution shared in
StackOverflow[1]. Note that accepting escape characters will void the
need to nullEventDelimiterEnabled flag. To sum it up,
1. May I have your blessing for using the StackOverflow snippet?
2. Is it okay with regards to licensing?
[1] https://stackoverflow.com/a/4298836/1278899
On Sun, Jul 12, 2020 at 7:35 PM Ralph Goers <ra...@dslextreme.com> wrote:
>
> Maybe. But it isn’t obvious and would have to be explicitly documented.
>
> Ralph
>
> > On Jul 12, 2020, at 8:09 AM, Matt Sicker <bo...@gmail.com> wrote:
> >
> > Wouldn’t that be � in XML?
> >
> > On Sat, Jul 11, 2020 at 16:26 Ralph Goers <ra...@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ı <vo...@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ı <vo...@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 <ra...@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ı <vo...@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.goers@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 <ra...@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.goers@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 <bo...@gmail.com>
>
>
Re: Testing Json Template Layout
Posted by Ralph Goers <ra...@dslextreme.com>.
Maybe. But it isn’t obvious and would have to be explicitly documented.
Ralph
> On Jul 12, 2020, at 8:09 AM, Matt Sicker <bo...@gmail.com> wrote:
>
> Wouldn’t that be � in XML?
>
> On Sat, Jul 11, 2020 at 16:26 Ralph Goers <ra...@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ı <vo...@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ı <vo...@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 <ra...@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ı <vo...@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.goers@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 <ra...@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.goers@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 <bo...@gmail.com>
Re: Testing Json Template Layout
Posted by Matt Sicker <bo...@gmail.com>.
Wouldn’t that be � in XML?
On Sat, Jul 11, 2020 at 16:26 Ralph Goers <ra...@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ı <vo...@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ı <vo...@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 <ra...@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ı <vo...@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.goers@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 <ra...@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.goers@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 <bo...@gmail.com>
Re: Testing Json Template Layout
Posted by Ralph Goers <ra...@dslextreme.com>.
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ı <vo...@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ı <vo...@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 <ra...@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ı <vo...@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 <ra...@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 <ra...@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 <ra...@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
>>>>>>>
>>>>>>
>>>>>>
>>>>>>
>>>>>
>>>>>
>>>>
>>>
>>>
>
Re: Testing Json Template Layout
Posted by Volkan Yazıcı <vo...@gmail.com>.
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ı <vo...@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 <ra...@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ı <vo...@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 <ra...@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 <ra...@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 <ra...@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
> > >>>>
> > >>>
> > >>>
> > >>>
> > >>
> > >>
> > >
> >
> >
Re: Testing Json Template Layout
Posted by Volkan Yazıcı <vo...@gmail.com>.
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 <ra...@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ı <vo...@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 <ra...@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 <ra...@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 <ra...@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
> >>>>
> >>>
> >>>
> >>>
> >>
> >>
> >
>
>
Re: Testing Json Template Layout
Posted by Ralph Goers <ra...@dslextreme.com>.
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ı <vo...@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 <ra...@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 <ra...@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 <ra...@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
>>>>
>>>
>>>
>>>
>>
>>
>
Re: Testing Json Template Layout
Posted by Volkan Yazıcı <vo...@gmail.com>.
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 <ra...@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 <ra...@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 <ra...@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
> >>
> >
> >
> >
>
>
Re: Testing Json Template Layout
Posted by Ralph Goers <ra...@dslextreme.com>.
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 <ra...@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 <ra...@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
>>
>
>
>
Re: Testing Json Template Layout
Posted by Ralph Goers <ra...@dslextreme.com>.
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 <ra...@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
>
Re: Adding type attribute to KeyValuePair (Was: Testing Json Template Layout)
Posted by Gary Gregory <ga...@gmail.com>.
Hi Volkan,
I don't want to take the time to dig into the details but I am glad you
found a resolution. My main concern is for binary compatibility of our
call-site facing APIs, so breaking file formats is ok by me in this case.
Gary
On Thu, Jan 28, 2021, 05:59 Volkan Yazıcı <vo...@gmail.com> wrote:
> Gary, check out the mess I caused by not accepting your proposal for
> renaming "type" to "format": LOG4J2-2973
> <https://issues.apache.org/jira/browse/LOG4J2-2973>. "type" conflicts with
> the array parsing semantics in properties file parser. Renaming "type" to
> "format" will allow us to read EventTemplateAdditionalField[] from
> properties files. While this is *not* a backward compatible change,
> injection of JSON template layout eventTemplateAdditionalFields has already
> been broken in release 2.14.0 and the introduced fix has already broken the
> backward compatibility. See LOG4J2-2961
> <https://issues.apache.org/jira/browse/LOG4J2-2961> for details. Hence, it
> should be okay.
>
> On Tue, Jul 14, 2020 at 11:59 PM Gary Gregory <ga...@gmail.com>
> wrote:
>
> > On Tue, Jul 14, 2020 at 1:55 PM Ralph Goers <ra...@dslextreme.com>
> > wrote:
> >
> > > Right now the type field in EventTemplateAdditionalField is an enum
> with
> > > possible values of JSON or STRING.
> > >
> >
> > This feels misnamed to me, JSON vs STRING is not a "type", JSON is a
> > "format", as in JSON vs. XML, and STRING could be a "type" only in the
> > free-form-string sense (to my mind again.)
> > So maybe this is a "format" choice of JSON vs. ANY_STRING/
> >
> > 2c,
> > Gary
> >
> >
> > > Ralph
> > >
> > > > On Jul 14, 2020, at 7:43 AM, Gary Gregory <ga...@gmail.com>
> > > wrote:
> > > >
> > > > Hi All,
> > > >
> > > > There are three types of solutions I see to these kinds of wants,
> > > depending
> > > > of what the domain of the 'type' is:
> > > > - KeyValuePair -> KeyValuePair<K,V> or KeyValuePair<V> and/or
> > > > - Add a Class<?> instance variable to denote the type, the hack
> usually
> > > > reserved to workaround type erasure, or
> > > > - Add something else like a String type ivar, which is what? A
> > free-form
> > > > string?
> > > >
> > > > Gary
> > > >
> > > > On Tue, Jul 14, 2020 at 10:24 AM Volkan Yazıcı <
> > volkan.yazici@gmail.com>
> > > > wrote:
> > > >
> > > >> Adding a "String type" field sounds pretty generic to me. What do
> you
> > > >> mean by "generic"?
> > > >>
> > > >> On Tue, Jul 14, 2020 at 4:10 PM Gary Gregory <
> garydgregory@gmail.com>
> > > >> wrote:
> > > >>>
> > > >>> On Tue, Jul 14, 2020 at 8:51 AM Volkan Yazıcı <
> > volkan.yazici@gmail.com
> > > >
> > > >>> wrote:
> > > >>>
> > > >>>> May I add a "type" (of type String) parameter to KeyValuePair? Do
> > you
> > > >>>> have any objections?
> > > >>>>
> > > >>>
> > > >>> Would we want to make this a generic?
> > > >>>
> > > >>> Gary
> > > >>>
> > > >>>
> > > >>>>
> > > >>>> On Mon, Jul 6, 2020 at 8:01 AM Ralph Goers <
> > > ralph.goers@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
> > > >>>>
> > > >>
> > >
> > >
> > >
> >
>
Re: Adding type attribute to KeyValuePair (Was: Testing Json Template Layout)
Posted by Volkan Yazıcı <vo...@gmail.com>.
Gary, check out the mess I caused by not accepting your proposal for
renaming "type" to "format": LOG4J2-2973
<https://issues.apache.org/jira/browse/LOG4J2-2973>. "type" conflicts with
the array parsing semantics in properties file parser. Renaming "type" to
"format" will allow us to read EventTemplateAdditionalField[] from
properties files. While this is *not* a backward compatible change,
injection of JSON template layout eventTemplateAdditionalFields has already
been broken in release 2.14.0 and the introduced fix has already broken the
backward compatibility. See LOG4J2-2961
<https://issues.apache.org/jira/browse/LOG4J2-2961> for details. Hence, it
should be okay.
On Tue, Jul 14, 2020 at 11:59 PM Gary Gregory <ga...@gmail.com>
wrote:
> On Tue, Jul 14, 2020 at 1:55 PM Ralph Goers <ra...@dslextreme.com>
> wrote:
>
> > Right now the type field in EventTemplateAdditionalField is an enum with
> > possible values of JSON or STRING.
> >
>
> This feels misnamed to me, JSON vs STRING is not a "type", JSON is a
> "format", as in JSON vs. XML, and STRING could be a "type" only in the
> free-form-string sense (to my mind again.)
> So maybe this is a "format" choice of JSON vs. ANY_STRING/
>
> 2c,
> Gary
>
>
> > Ralph
> >
> > > On Jul 14, 2020, at 7:43 AM, Gary Gregory <ga...@gmail.com>
> > wrote:
> > >
> > > Hi All,
> > >
> > > There are three types of solutions I see to these kinds of wants,
> > depending
> > > of what the domain of the 'type' is:
> > > - KeyValuePair -> KeyValuePair<K,V> or KeyValuePair<V> and/or
> > > - Add a Class<?> instance variable to denote the type, the hack usually
> > > reserved to workaround type erasure, or
> > > - Add something else like a String type ivar, which is what? A
> free-form
> > > string?
> > >
> > > Gary
> > >
> > > On Tue, Jul 14, 2020 at 10:24 AM Volkan Yazıcı <
> volkan.yazici@gmail.com>
> > > wrote:
> > >
> > >> Adding a "String type" field sounds pretty generic to me. What do you
> > >> mean by "generic"?
> > >>
> > >> On Tue, Jul 14, 2020 at 4:10 PM Gary Gregory <ga...@gmail.com>
> > >> wrote:
> > >>>
> > >>> On Tue, Jul 14, 2020 at 8:51 AM Volkan Yazıcı <
> volkan.yazici@gmail.com
> > >
> > >>> wrote:
> > >>>
> > >>>> May I add a "type" (of type String) parameter to KeyValuePair? Do
> you
> > >>>> have any objections?
> > >>>>
> > >>>
> > >>> Would we want to make this a generic?
> > >>>
> > >>> Gary
> > >>>
> > >>>
> > >>>>
> > >>>> On Mon, Jul 6, 2020 at 8:01 AM Ralph Goers <
> > ralph.goers@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
> > >>>>
> > >>
> >
> >
> >
>
Re: Adding type attribute to KeyValuePair (Was: Testing Json Template
Layout)
Posted by Ralph Goers <ra...@dslextreme.com>.
FWIW, this is one area Log4j could use improvement as the way it is done is too brittle. Ideally the Layout should be able to specify the format for all messages to use in a generic way. I don’t believe that is the case today. I am also sure user’s would like to be able to customize that to some degree.
Ralph
> On Jul 14, 2020, at 3:44 PM, Ralph Goers <ra...@dslextreme.com> wrote:
>
> That is true. There is at least one other place where we use “format” to differentiate between JSON, XML, and Java. See MapMessage.
>
> Ralph
>
>> On Jul 14, 2020, at 11:06 AM, Gary Gregory <ga...@gmail.com> wrote:
>>
>> On Tue, Jul 14, 2020 at 1:55 PM Ralph Goers <ra...@dslextreme.com>
>> wrote:
>>
>>> Right now the type field in EventTemplateAdditionalField is an enum with
>>> possible values of JSON or STRING.
>>>
>>
>> This feels misnamed to me, JSON vs STRING is not a "type", JSON is a
>> "format", as in JSON vs. XML, and STRING could be a "type" only in the
>> free-form-string sense (to my mind again.)
>> So maybe this is a "format" choice of JSON vs. ANY_STRING/
>>
>> 2c,
>> Gary
>>
>>
>>> Ralph
>>>
>>>> On Jul 14, 2020, at 7:43 AM, Gary Gregory <ga...@gmail.com>
>>> wrote:
>>>>
>>>> Hi All,
>>>>
>>>> There are three types of solutions I see to these kinds of wants,
>>> depending
>>>> of what the domain of the 'type' is:
>>>> - KeyValuePair -> KeyValuePair<K,V> or KeyValuePair<V> and/or
>>>> - Add a Class<?> instance variable to denote the type, the hack usually
>>>> reserved to workaround type erasure, or
>>>> - Add something else like a String type ivar, which is what? A free-form
>>>> string?
>>>>
>>>> Gary
>>>>
>>>> On Tue, Jul 14, 2020 at 10:24 AM Volkan Yazıcı <vo...@gmail.com>
>>>> wrote:
>>>>
>>>>> Adding a "String type" field sounds pretty generic to me. What do you
>>>>> mean by "generic"?
>>>>>
>>>>> On Tue, Jul 14, 2020 at 4:10 PM Gary Gregory <ga...@gmail.com>
>>>>> wrote:
>>>>>>
>>>>>> On Tue, Jul 14, 2020 at 8:51 AM Volkan Yazıcı <volkan.yazici@gmail.com
>>>>
>>>>>> wrote:
>>>>>>
>>>>>>> May I add a "type" (of type String) parameter to KeyValuePair? Do you
>>>>>>> have any objections?
>>>>>>>
>>>>>>
>>>>>> Would we want to make this a generic?
>>>>>>
>>>>>> Gary
>>>>>>
>>>>>>
>>>>>>>
>>>>>>> On Mon, Jul 6, 2020 at 8:01 AM Ralph Goers <
>>> ralph.goers@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
>>>>>>>
>>>>>
>>>
>>>
>>>
>
Re: Adding type attribute to KeyValuePair (Was: Testing Json Template
Layout)
Posted by Ralph Goers <ra...@dslextreme.com>.
That is true. There is at least one other place where we use “format” to differentiate between JSON, XML, and Java. See MapMessage.
Ralph
> On Jul 14, 2020, at 11:06 AM, Gary Gregory <ga...@gmail.com> wrote:
>
> On Tue, Jul 14, 2020 at 1:55 PM Ralph Goers <ra...@dslextreme.com>
> wrote:
>
>> Right now the type field in EventTemplateAdditionalField is an enum with
>> possible values of JSON or STRING.
>>
>
> This feels misnamed to me, JSON vs STRING is not a "type", JSON is a
> "format", as in JSON vs. XML, and STRING could be a "type" only in the
> free-form-string sense (to my mind again.)
> So maybe this is a "format" choice of JSON vs. ANY_STRING/
>
> 2c,
> Gary
>
>
>> Ralph
>>
>>> On Jul 14, 2020, at 7:43 AM, Gary Gregory <ga...@gmail.com>
>> wrote:
>>>
>>> Hi All,
>>>
>>> There are three types of solutions I see to these kinds of wants,
>> depending
>>> of what the domain of the 'type' is:
>>> - KeyValuePair -> KeyValuePair<K,V> or KeyValuePair<V> and/or
>>> - Add a Class<?> instance variable to denote the type, the hack usually
>>> reserved to workaround type erasure, or
>>> - Add something else like a String type ivar, which is what? A free-form
>>> string?
>>>
>>> Gary
>>>
>>> On Tue, Jul 14, 2020 at 10:24 AM Volkan Yazıcı <vo...@gmail.com>
>>> wrote:
>>>
>>>> Adding a "String type" field sounds pretty generic to me. What do you
>>>> mean by "generic"?
>>>>
>>>> On Tue, Jul 14, 2020 at 4:10 PM Gary Gregory <ga...@gmail.com>
>>>> wrote:
>>>>>
>>>>> On Tue, Jul 14, 2020 at 8:51 AM Volkan Yazıcı <volkan.yazici@gmail.com
>>>
>>>>> wrote:
>>>>>
>>>>>> May I add a "type" (of type String) parameter to KeyValuePair? Do you
>>>>>> have any objections?
>>>>>>
>>>>>
>>>>> Would we want to make this a generic?
>>>>>
>>>>> Gary
>>>>>
>>>>>
>>>>>>
>>>>>> On Mon, Jul 6, 2020 at 8:01 AM Ralph Goers <
>> ralph.goers@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
>>>>>>
>>>>
>>
>>
>>
Re: Adding type attribute to KeyValuePair (Was: Testing Json Template Layout)
Posted by Gary Gregory <ga...@gmail.com>.
On Tue, Jul 14, 2020 at 1:55 PM Ralph Goers <ra...@dslextreme.com>
wrote:
> Right now the type field in EventTemplateAdditionalField is an enum with
> possible values of JSON or STRING.
>
This feels misnamed to me, JSON vs STRING is not a "type", JSON is a
"format", as in JSON vs. XML, and STRING could be a "type" only in the
free-form-string sense (to my mind again.)
So maybe this is a "format" choice of JSON vs. ANY_STRING/
2c,
Gary
> Ralph
>
> > On Jul 14, 2020, at 7:43 AM, Gary Gregory <ga...@gmail.com>
> wrote:
> >
> > Hi All,
> >
> > There are three types of solutions I see to these kinds of wants,
> depending
> > of what the domain of the 'type' is:
> > - KeyValuePair -> KeyValuePair<K,V> or KeyValuePair<V> and/or
> > - Add a Class<?> instance variable to denote the type, the hack usually
> > reserved to workaround type erasure, or
> > - Add something else like a String type ivar, which is what? A free-form
> > string?
> >
> > Gary
> >
> > On Tue, Jul 14, 2020 at 10:24 AM Volkan Yazıcı <vo...@gmail.com>
> > wrote:
> >
> >> Adding a "String type" field sounds pretty generic to me. What do you
> >> mean by "generic"?
> >>
> >> On Tue, Jul 14, 2020 at 4:10 PM Gary Gregory <ga...@gmail.com>
> >> wrote:
> >>>
> >>> On Tue, Jul 14, 2020 at 8:51 AM Volkan Yazıcı <volkan.yazici@gmail.com
> >
> >>> wrote:
> >>>
> >>>> May I add a "type" (of type String) parameter to KeyValuePair? Do you
> >>>> have any objections?
> >>>>
> >>>
> >>> Would we want to make this a generic?
> >>>
> >>> Gary
> >>>
> >>>
> >>>>
> >>>> On Mon, Jul 6, 2020 at 8:01 AM Ralph Goers <
> ralph.goers@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
> >>>>
> >>
>
>
>
Re: Adding type attribute to KeyValuePair (Was: Testing Json Template
Layout)
Posted by Ralph Goers <ra...@dslextreme.com>.
Right now the type field in EventTemplateAdditionalField is an enum with possible values of JSON or STRING.
Ralph
> On Jul 14, 2020, at 7:43 AM, Gary Gregory <ga...@gmail.com> wrote:
>
> Hi All,
>
> There are three types of solutions I see to these kinds of wants, depending
> of what the domain of the 'type' is:
> - KeyValuePair -> KeyValuePair<K,V> or KeyValuePair<V> and/or
> - Add a Class<?> instance variable to denote the type, the hack usually
> reserved to workaround type erasure, or
> - Add something else like a String type ivar, which is what? A free-form
> string?
>
> Gary
>
> On Tue, Jul 14, 2020 at 10:24 AM Volkan Yazıcı <vo...@gmail.com>
> wrote:
>
>> Adding a "String type" field sounds pretty generic to me. What do you
>> mean by "generic"?
>>
>> On Tue, Jul 14, 2020 at 4:10 PM Gary Gregory <ga...@gmail.com>
>> wrote:
>>>
>>> On Tue, Jul 14, 2020 at 8:51 AM Volkan Yazıcı <vo...@gmail.com>
>>> wrote:
>>>
>>>> May I add a "type" (of type String) parameter to KeyValuePair? Do you
>>>> have any objections?
>>>>
>>>
>>> Would we want to make this a generic?
>>>
>>> Gary
>>>
>>>
>>>>
>>>> On Mon, Jul 6, 2020 at 8:01 AM Ralph Goers <ralph.goers@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
>>>>
>>
Re: Adding type attribute to KeyValuePair (Was: Testing Json Template Layout)
Posted by Jochen Wiedmann <jo...@gmail.com>.
On Wed, Jul 15, 2020 at 10:06 AM Volkan Yazıcı <vo...@gmail.com> wrote:
>
> The reason I wanted to go with a simple `String type` field is to keep
> the impact surface and backward compatibility issues at the bay.
>
> I am in favor of `KeyValuePair<V>` -- `KeyValuePair<K,V>` feels like
> an engineering overkill for our usages. That said, this translates to
> 368 occurrences of `KeyValuePair` in the code that needs to be
> adjusted. Do we really want to go in that direction?
Would it work, to introduce a new GenericKeyValuePair<V>, and change
KeyValuePair to extend GenericKeyValuePair<String>?
Jochen
--
Look, that's why there's rules, understand? So that you think before
you break 'em.
-- (Terry Pratchett, Thief of Time)
Re: Adding type attribute to KeyValuePair (Was: Testing Json Template Layout)
Posted by Volkan Yazıcı <vo...@gmail.com>.
The reason I wanted to go with a simple `String type` field is to keep
the impact surface and backward compatibility issues at the bay.
I am in favor of `KeyValuePair<V>` -- `KeyValuePair<K,V>` feels like
an engineering overkill for our usages. That said, this translates to
368 occurrences of `KeyValuePair` in the code that needs to be
adjusted. Do we really want to go in that direction?
On Tue, Jul 14, 2020 at 4:44 PM Gary Gregory <ga...@gmail.com> wrote:
>
> Hi All,
>
> There are three types of solutions I see to these kinds of wants, depending
> of what the domain of the 'type' is:
> - KeyValuePair -> KeyValuePair<K,V> or KeyValuePair<V> and/or
> - Add a Class<?> instance variable to denote the type, the hack usually
> reserved to workaround type erasure, or
> - Add something else like a String type ivar, which is what? A free-form
> string?
>
> Gary
>
> On Tue, Jul 14, 2020 at 10:24 AM Volkan Yazıcı <vo...@gmail.com>
> wrote:
>
> > Adding a "String type" field sounds pretty generic to me. What do you
> > mean by "generic"?
> >
> > On Tue, Jul 14, 2020 at 4:10 PM Gary Gregory <ga...@gmail.com>
> > wrote:
> > >
> > > On Tue, Jul 14, 2020 at 8:51 AM Volkan Yazıcı <vo...@gmail.com>
> > > wrote:
> > >
> > > > May I add a "type" (of type String) parameter to KeyValuePair? Do you
> > > > have any objections?
> > > >
> > >
> > > Would we want to make this a generic?
> > >
> > > Gary
> > >
> > >
> > > >
> > > > On Mon, Jul 6, 2020 at 8:01 AM Ralph Goers <ralph.goers@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
> > > >
> >
Re: Adding type attribute to KeyValuePair (Was: Testing Json Template Layout)
Posted by Gary Gregory <ga...@gmail.com>.
Hi All,
There are three types of solutions I see to these kinds of wants, depending
of what the domain of the 'type' is:
- KeyValuePair -> KeyValuePair<K,V> or KeyValuePair<V> and/or
- Add a Class<?> instance variable to denote the type, the hack usually
reserved to workaround type erasure, or
- Add something else like a String type ivar, which is what? A free-form
string?
Gary
On Tue, Jul 14, 2020 at 10:24 AM Volkan Yazıcı <vo...@gmail.com>
wrote:
> Adding a "String type" field sounds pretty generic to me. What do you
> mean by "generic"?
>
> On Tue, Jul 14, 2020 at 4:10 PM Gary Gregory <ga...@gmail.com>
> wrote:
> >
> > On Tue, Jul 14, 2020 at 8:51 AM Volkan Yazıcı <vo...@gmail.com>
> > wrote:
> >
> > > May I add a "type" (of type String) parameter to KeyValuePair? Do you
> > > have any objections?
> > >
> >
> > Would we want to make this a generic?
> >
> > Gary
> >
> >
> > >
> > > On Mon, Jul 6, 2020 at 8:01 AM Ralph Goers <ralph.goers@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
> > >
>
Re: Adding type attribute to KeyValuePair (Was: Testing Json Template Layout)
Posted by Volkan Yazıcı <vo...@gmail.com>.
Adding a "String type" field sounds pretty generic to me. What do you
mean by "generic"?
On Tue, Jul 14, 2020 at 4:10 PM Gary Gregory <ga...@gmail.com> wrote:
>
> On Tue, Jul 14, 2020 at 8:51 AM Volkan Yazıcı <vo...@gmail.com>
> wrote:
>
> > May I add a "type" (of type String) parameter to KeyValuePair? Do you
> > have any objections?
> >
>
> Would we want to make this a generic?
>
> Gary
>
>
> >
> > On Mon, Jul 6, 2020 at 8:01 AM Ralph Goers <ra...@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
> >
Re: Adding type attribute to KeyValuePair (Was: Testing Json Template Layout)
Posted by Gary Gregory <ga...@gmail.com>.
On Tue, Jul 14, 2020 at 8:51 AM Volkan Yazıcı <vo...@gmail.com>
wrote:
> May I add a "type" (of type String) parameter to KeyValuePair? Do you
> have any objections?
>
Would we want to make this a generic?
Gary
>
> On Mon, Jul 6, 2020 at 8:01 AM Ralph Goers <ra...@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
>
Adding type attribute to KeyValuePair (Was: Testing Json Template Layout)
Posted by Volkan Yazıcı <vo...@gmail.com>.
May I add a "type" (of type String) parameter to KeyValuePair? Do you
have any objections?
On Mon, Jul 6, 2020 at 8:01 AM Ralph Goers <ra...@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
Re: Testing Json Template Layout
Posted by Apache <ra...@dslextreme.com>.
Yes, please do.
Ralph
> On Jul 22, 2020, at 6:22 AM, Volkan Yazıcı <vo...@gmail.com> wrote:
>
> Ralph, I am fine with the current state of the plugin. I don't intend to
> have further improvements in the short-term regarding KeyValuePair
> enhancements and/or escape character handling. If you are okay with it, I
> want to start rebasing this onto 2.x branch.
>
>> On Mon, Jul 6, 2020 at 8:01 AM Ralph Goers <ra...@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
>>
Re: Testing Json Template Layout
Posted by Volkan Yazıcı <vo...@gmail.com>.
Ralph, I am fine with the current state of the plugin. I don't intend to
have further improvements in the short-term regarding KeyValuePair
enhancements and/or escape character handling. If you are okay with it, I
want to start rebasing this onto 2.x branch.
On Mon, Jul 6, 2020 at 8:01 AM Ralph Goers <ra...@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
>