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