udmi icon indicating copy to clipboard operation
udmi copied to clipboard

Log level mismatch between documentation and Sequencer tool

Open DollyGodet opened this issue 3 years ago • 4 comments

In the documentation, it is specified that the level of a log message with category system.config.receive should be Level.DEBUG.

In the Sequencer tool (com.google.daq.mqtt.validator.validations.ConfigValidator#broken_config), broken_config test validates that it received a log message with a system.config.receive category and a Level.INFO level. This does not match the documentation.

There is other similar mismatches :

  • broken_config test validates that it received a log message with category system.config.apply and Level.INFO level, but the documentation specified that it should be a Level.NOTICE level.
  • broken_config test validates that it has receive a log message with category system.config.parse and Level.INFO level, but the documentation specified that it should be a Level.DEBUG level.
  • extra_config test has the same mismatches.

DollyGodet avatar Jul 07 '22 14:07 DollyGodet

@grafnu @DollyGodet @pisuke any views as to which should be correct?

In my mind, INFO for all the config apply/parse/receive events was excessive, and I'm leaning towards DEBUG or NOTICE log level for receive & successful parse events, and then INFO for config apply, which matches what is in the documentation.

The reason for this is the state message should already indirectly communicate this, and to avoid alert overload

Maybe parse could be replaced with config apply error?

noursaidi avatar Jul 12 '22 13:07 noursaidi

Hi @noursaidi I also agree it makes sense to have INFO for config apply. I think that receive should be NOTICE and parse should be DEBUG.

pisuke avatar Jul 12 '22 13:07 pisuke

I'm not exactly sure what you're saying, but the ordering goes DEBUG < INFO < NOTICE

With that, the statement "INFO for all the config apply/parse/receive events was excessive, and I'm leaning towards DEBUG or NOTICE log level for receive & successful parse events" doesn't quite make sense to me.

Nobody is going to alert on INFO, I doubt anybody would alert on NOTICE either... that's just more "if you're paying attention to this here's something to look for". INFO messages pretty much get lots in the noise but are useful to have around by default for triage. DEBUG is only if you're a dev actively working on a problem. With all this, I'd recommend:

  • receive: INFO -- kinda want to know it happens, but not really significant.
  • parse: DEBUG -- only something to pay attention to if it actually goes wrong
  • apply: NOTICE -- the "main event" of this... this implies everything else was successful

Any deviation from "as expected" would be then be warning/error as appropriate.

On Tue, Jul 12, 2022 at 6:28 AM Francesco Anselmo @.***> wrote:

Hi @noursaidi https://github.com/noursaidi I also agree it makes sense to have INFO for config apply. I think that receive should be NOTICE and parse should be DEBUG.

— Reply to this email directly, view it on GitHub https://github.com/faucetsdn/udmi/issues/386#issuecomment-1181761551, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAIEPD4354P2QYNVVR5A3ADVTVXIZANCNFSM525TOVCQ . You are receiving this because you were mentioned.Message ID: @.***>

grafnu avatar Jul 12 '22 14:07 grafnu

I confused the levels of NOTICE and INFO! The levels you've suggested sound good to me

noursaidi avatar Jul 12 '22 15:07 noursaidi