RE: [PATCH] trace: take live traces via telemetry

["Morten Brørup" <mb@smartsharesystems.com>]

> From: David Marchand [mailto:david.marchand@redhat.com]
> Sent: Wednesday, 19 October 2022 09.39
> 
> Hello Morten, Bruce,
> 
> On Tue, Oct 18, 2022 at 4:34 PM Morten Brørup
> <mb@smartsharesystems.com> wrote:
> > > --> /trace/list,lib.ethdev.*
> > > {"/trace/list": {"lib.ethdev.configure": "Disabled",
> > >     "lib.ethdev.rxq.setup": "Disabled",
> > >     "lib.ethdev.txq.setup": "Disabled",
> > >     "lib.ethdev.start": "Disabled",
> > >     "lib.ethdev.stop": "Disabled",
> > >     "lib.ethdev.close": "Disabled",
> > >     "lib.ethdev.rx.burst": "Disabled",
> > >     "lib.ethdev.tx.burst": "Disabled"}}
> >
> > Jerin commented that "Disabled"/"Enabled" are a bit verbose, and
> suggested shortening them.
> 
> I forgot to handle this part (EINTR...), thanks for reminding.
> 
> 
> > It seems to me that these values are Boolean, and should be true or
> false (not surrounded by quotation marks), instead of some string
> representing a Boolean value. Note: This would require expanding the
> telemetry library with a Boolean type.
> 
> Indeed.
> 
> >
> > Alternatively, use integer values 0 or 1.
> >
> > If we want to represent Boolean values as strings, I vote for "TRUE"
> and "FALSE", using all upper case to indicate that they are magic
> strings - and also to help avoid confusion with the JSON true/false
> Boolean values, which are all lower case.
> 
> Introducing those strings is confusing, especially if we later
> introduce the boolean type.
> I had a quick try and adding the boolean type is relatively easy (I
> just posted a patch, see
> https://patchwork.dpdk.org/project/dpdk/patch/20221019073702.3948624-1-
> david.marchand@redhat.com/).
> 
> I would either go with adding this new type (though we are past rc1,
> this addition is self contained and low risk), or use simple integers.

+1 to adding this new type.

It might be wider ranging, though:

Are other existing telemetry data in fact Boolean, but currently using some other type, and should switch to using the new Boolean type too?

> 
> 
> --
> David Marchand
>