added silly and critical trace levels

[jupe]

Status

READY/HOLD

Description

added silly and critical trace levels. Replace bitmask debug levels as number.

New API's

    tr_silly("silly print");
    tr_critical("fatal print");

critical traces are colored with red background, silly traces are with same colors than debug traces (gray).

Related issues:

Fixes #79

Todo

• Implementation
• Test
• Doc
• Decision about merging

[teetak01]

Looks good.

[jupe]

tr_silly could cause conflict if somebody define it after including this header but maybe we shouldn’t do any more hacks in here

[teetak01]

This might require some testing as this seems to break the tracing of existing applications.

[jupe]

could you point what breaks so I could fix it?

[kjbracey]

Basically fine with the change, but I would vaguely prefer to make better use of higher-than-debug levels. That would be harder work though. See comment on issue #79

[teetak01]

Some of the traces are not visible with internal test application.

[JanneKiiskila]

Should it be just called DEEP rather than SILLY?

[jupe]

I'm fine with both keys. I just pick silly keyword from winston library :) Votes ?

[jupe]

I've updated PR and covered all comments, except these:

Should it be just called DEEP rather than SILLY?

Any opinions ? tr_deep() or tr_silly() ?

Most of our infos should be notice, and a lot of our debug should be info.

Another approach is to drop whole PR and change our code base so that traces are more relevant level... -> Question are: Do we really need more verbose levels ? Can't we use existing levels better ?

[SeppoTakalo]

We are already in problem with too much traces and adding more levels is just encouraging people to trace more, and trace even silly things...

So.. I'm not sure this is good thing.

What exactly are the differences between tr_warn() and tr_critical()?

[jupe]

We are already in problem with too much traces and adding more levels is just encouraging people to trace more, and trace even silly things...

True. Can we survive with existing trace levels or not? @teetak01 can you give more details why you asked #79 originally ? What was the actual problem ?

So.. I'm not sure this is good thing.

I've same feeling..

What exactly are the differences between tr_warn() and tr_critical()?

depends on how we want to use it, but I would guess that warning means something like something wrong happens but code can survive with it, and critical is something more dramatical that something might not work anymore as expected...

[teetak01]

Well, I think there would be some benefit of being able to seperate some debug-level tracing to silly. Of course I agree that @SeppoTakalo concern is probably valid. Silly-like level could be used for extensive diagnostic output (including IN/OUT tracing) when we know something is broken, but only can ask customer to provide some logs. But if you think that this is more trouble than benefit, the issue can also be rejected.

I do not see much benefit for tr_critical(). For Mbed OS this would effectively be MBED_ERROR(), and in case of Client, in such fatal cases, we could just call platform-level reboot, if we would think that the library could go to such broken state we cannot recover anymore.

[SeppoTakalo]

Even if warning and error are logically different, I still don't see the need to have those as separate debug levels but I'm not hardly against, if you see the need.

I would be happy with 3 levels: debug, info, warning.
Even with those, its sometimes hard for developer to device which level the message belongs to.

Syslog seems to have 7 severity levels.

I would prefer to unify our levels to match those.

[JanneKiiskila]

There should be some guidelines in telling people what log level is meant for that, preferrably with some good examples. Easier said than done, though. :-)

[teetak01]

Well, the syslog seems to have sensible description for different levels, and are mostly in line how mbed-trace currently is used.

[kjbracey]

Strongly agree with syslog consistency. As I said in #79, adding silly is effectively a backwards-compatible attempt to deal with our overuse of "debug", but it locks in our not-syslogness. Most of our traces should be higher level, really. (Quoting from myself: "Most of our infos should be notice, and a lot of our debug should be info.")

[kjbracey]

Note that in Linux, you would never just turn on "debug" for the whole system - it's very much a per-component thing. Whole system would bring it to its knees. Which suggests it is very much "silly"...

[teetak01]

mbed-trace currently has only a limited functionality for controlling component-specific tracing at compile-time #59

[kjbracey]

mbed-trace currently has only a limited functionality for controlling component-specific tracing at compile-time

Had some discussion back in April with @jupe about this, following #76. Whatever your "maximum" trace level is, be it "debug" or "silly", you really do want to have it compile-time per-component selectable in some way.

The lack of that is currently forcing local instances of

#ifdef MY_EXTRA_TRACE       (or often #if 0)
#define tr_silly(...) tr_debug(__VA_ARGS__)
#else
#define tr_silly(...) (void)0
#endif

I was starting to think about how to do that generally with token pasting nonsense - something like JSON:

"trace-debug-enable": { "MyLb", "Cmp3", "lwIP" }

leading to C defines TRACE_ENABLE_MyLb=y, TRACE_ENABLE_Cmp3=y, TRACE_ENABLE_lwIP=y.

Then in "mbed_trace.h" start pasting tr_debug_##(TRACE_ENABLE_##TRACE_GROUP) through whatever layers to nest that correctly so you get tr_debug_y (defined as tr_debug) or tr_debug_n (defined as no-op).

Only got as far as hand-waving about it though.

[jupe]

I would vote to drop these new trace levels (=reject #80 and #79, focus component based trace activation feature that @kjbracey-arm mentioned and update existing traces more appropriate levels. Is it okay for others ? This of course causes some work for teams to review component traces and re-think trace levels they uses.. Maybe that is not bad idea afterall - I'm pretty sure that there is some cleanup to do anyway..