Re: Adding type attribute to KeyValuePair (Was: Testing Json Template Layout)

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

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