PelionIoT / mbed-trace

mbed trace library
Apache License 2.0
18 stars 15 forks source link

Add more log-levels #79

Closed teetak01 closed 2 years ago

teetak01 commented 6 years ago

The current log-levels are not sufficient. We could have something even more verbose than normal tr_debug.

Generally it might be useful to add more resolution for all levels.

tommikas commented 6 years ago

Something like python logger logging levels might be nice. For the most part that should be possible to do in a backwards compatible way too, as long as everyone's been using the macros.

@jupe, what do you think?

ciarmcom commented 6 years ago

ARM Internal Ref: IOTSYST-2734

jupe commented 6 years ago

actually I’ve seen tr_silly in use somewhere in codebase - if we bring something like that it might breaks compiling. Default log level shouldn’t be any more verbose - otherwise it would need more memory and more serial port traffic. Generally there could be at least one more levels.

tommikas commented 6 years ago

I think we could give users the option of more levels without significant memory overhead or huge code changes. Serial port traffic is a concern, but simply adding more logging levels does not necessarily mean more lines, it just means more control over what is printed. Even with a tr_silly if the default log level is still debug there isn't necessarily any more traffic.

teetak01 commented 6 years ago

I was especially thinking about some low-level libraries we have. Those could do their "debug" level tracing in some "silly" level instead. Those should not appear on default settings, but could be enabled for ex. linux-builds where there should not be any issues with trace-leveling.

kjbracey commented 6 years ago

I think the problem is partly that people don't use "info" enough. Logging tends to be bunched too much at "debug" level, leaving nowhere for the really detailed stuff.

Compared to Linux, we're missing notice (between info and warning), and critical/alert/emergency above error. Their debug is pretty verbose - they have a mechanism to turn it on and off dynamically per file/function etc, and generally speaking turning on debug for the entire kernel causes enough output for the system to stop working, at least in Android - it is effectively "silly".

I'd kind of like to see the default log level set higher (to hide info and debug, say) and most messages - particularly those inspected by CLI tests - changed to be higher than they are. Most of our infos should be notice, and a lot of our debug should be info.

Thoughts? But I guess adding "silly" below "debug" is much more backwards-compatible, effectively making room.

jupe commented 6 years ago

I think the problem is partly that people don't use "info" enough.

I agree.

I'd kind of like to see the default log level set higher (to hide info and debug, say)

Agree. Should we instead add notice -level and use that level by default..hmm

tommikas commented 6 years ago

"notice" would be between "info" and "warning" though. But, it could be added without breaking backwards compatibility the way #80 is currently done, no?

kjbracey commented 6 years ago

The thing that wouldn't be backwards compatible would be raising the default log level, unless we went through to boost the level of all the things tests were relying on, and/or made sure the tests lowered from default.

As my ideal endpoint, I'd say I would say "show warnings" or "show notice" would be appropriate levels for default builds, but that CLI test apps should normally run with "show info". Normal runs and builds would not use/incorporate "debug".

jupe commented 6 years ago

all the things tests were relying on

tests should not rely on any traces. If that’s the case maybe it is okay to break them..At least I don’t think that there is many of them, or is there? 🙄

Yep, default build should be enough warn/notice. Test builds info or debug depends on case..

So, do we want to:

1) bring silly and critical levels 2) just notice between info and warn? 3) keep as it is now and pushing users to use more info... ?

default level changing could be done in another PR if/when needed..