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