py3status icon indicating copy to clipboard operation
py3status copied to clipboard

Set up standard logging module

Open rlerm opened this issue 2 years ago • 18 comments

Initial parts of https://github.com/ultrabug/py3status/issues/2063: Set up the root logger from the logging module. Make py3.log defer to that logger.

The root logger's configuration was built in a way that tries to preserve the existing behavior as much as possble.

rlerm avatar Sep 05 '21 17:09 rlerm

This pull request introduces 1 alert when merging ac081129d82cd8475da528222e2c8e5f3fc3803f into cafe0c7a1840d105ea585e16b95eda3ecd96c219 - view on LGTM.com

new alerts:

  • 1 for Unused import

lgtm-com[bot] avatar Sep 05 '21 17:09 lgtm-com[bot]

Thanks @rlerm, this is a very promising first contribution to this project :medal_sports:

When you say "initial parts" I guess you plan on adding things, is that correct? If so, please mark this PR as WIP and tell me when the time is right to review and test it please?

Also, you will want to demonstrate and document what this new logging system brings to the table and how to use it to achieve the goals you expressed in #2063 please

Thanks again

ultrabug avatar Sep 07 '21 19:09 ultrabug

This pull request introduces 2 alerts when merging 7736dfa6fa755a08a69581323bf5d8dbcdafd85c into cafe0c7a1840d105ea585e16b95eda3ecd96c219 - view on LGTM.com

new alerts:

  • 1 for Syntax error
  • 1 for Module is imported with 'import' and 'import from'

lgtm-com[bot] avatar Sep 20 '21 00:09 lgtm-com[bot]

Thanks! (And sorry for the delay in responding)

The initial PR was usable, but it was lacking a way to configure the logging in a more advanced way. I've added, that, and the documentation.

Please let me know what you think.

rlerm avatar Sep 20 '21 00:09 rlerm

Trying your PR locally

For now I get

py3status: Setup error (local variable 'logging' referenced before assignment). Please try to fix this and reload i3wm (Mod+Shift+R)

EDIT:

This is Python 3.9 I run

Traceback (most recent call last):
  File "/tmp/py3status/py3status/__init__.py", line 39, in main
    py3.setup()
  File "/tmp/py3status/py3status/core.py", line 576, in setup
    self._setup_logging()
  File "/tmp/py3status/py3status/core.py", line 552, in _setup_logging
    root = logging.getLogger(name=None)
UnboundLocalError: local variable 'logging' referenced before assignment

ultrabug avatar Oct 05 '21 19:10 ultrabug

That's weird. Do you have a line number? The logging module is being imported quite high up in the module.

rlerm avatar Oct 05 '21 19:10 rlerm

That's weird. Do you have a line number? The logging module is being imported quite high up in the module.

Yep, see edited comment

There must be a scope context that's not obvious and that's why we need to either global logging in the function or import it again inside I guess

ultrabug avatar Oct 05 '21 19:10 ultrabug

Even when bypassing this error, it looks like I get no logs in my syslog when running with the -d debug flag

ultrabug avatar Oct 05 '21 19:10 ultrabug

Thanks for catching this. I believe I've fixed both issues.

rlerm avatar Oct 05 '21 20:10 rlerm

So @rlerm thanks for the fix, and sorry for the delay

Tried it, does not crash :+1:

But then I tried your JSON example and I'm not sure it works as expected

{
    "version": 1,
    "handlers": {
        "file": {
            "class": "logging.handlers.RotatingFileHandler",
            "filename": "/tmp/py3status_log.log",
            "maxBytes": 2048,
            "formatter": "default"
        }
    },
    "formatters": {
        "default": {
            "validate": false,
            "format":"%(asctime)s %(levelname)s %(module)s %(message)s",
            "datefmt":"%Y-%m-%d %H:%M:%S"
        }
    },
    "loggers": {
        "root": {"handlers": ["file"], "level": "INFO"},
        "xrandr": {"handlers": ["file"], "level": "DEBUG"}
    }
}

I expected everything INFO logged to the file, and the xrandr module as DEBUG but I don't get anything else than a third party lib (google api) log output written on the file. journalctl goes silent as well...

Shouldn't we propose a default log-config json file that mimics the current behavior we have first?

ultrabug avatar Nov 07 '21 21:11 ultrabug

Thanks (and sorry for the delay).

You're right, the default Python behavior is to use disable_existing_loggers=True, which makes the "core" logger be disabled when we do the configuration. That was effectively disabling the "core" logger.

As for controlling the "xrandr" module, that won't work directly. Because it wasn't changed to create its own logger (logging.getLogger('xrandr')), it will get logged by the "core" logger. I'd expect people who want to use this logging facility to update their modules before trying to configure logging. (When I wrote this, I was working on a new module locally, and used the logging there)

It's expected that syslog gets silent with the config in the example, since that takes over the entire configuration.

So now the logs should be working even with the example in the doc, minus the "xrandr" part.

rlerm avatar Dec 17 '21 00:12 rlerm

I tried again and failed @rlerm I'm sorry but the documentation you wrote is not easy enough for stupid people like me...

Could you please:

  • convert the xrandr to logging, the way you intend it
  • make xrandr an example in your JSON config in the readme?
  • specify in the text that by using this, the core will be logged as INFO while xrandr as DEBUG in the /tmp/py3status_log.log file

?

ultrabug avatar Jan 05 '22 15:01 ultrabug

Thanks, that's a great suggestion. I've ported the module and updated the docs. I don't have a lot of stuff going on in my monitor setup to use everything the xrandr module, but I tested a few layout changes and I was able to get the logs in the expected file.

rlerm avatar Jan 05 '22 22:01 rlerm

This pull request introduces 1 alert when merging 27caf064af388e4bf150ee59e9d698f684206b0c into 5e2a3c16edf91b94081098de5c86fd6b4875638a - view on LGTM.com

new alerts:

  • 1 for Unused import

lgtm-com[bot] avatar Jan 31 '22 16:01 lgtm-com[bot]

Looks like my rebase broke your PR I'm sorry, could you fix it again? promise I'll get this in!

ultrabug avatar Jan 31 '22 16:01 ultrabug

I've done the rebase, can you check it again? Thanks!

rlerm avatar Jan 31 '22 23:01 rlerm

@rlerm as I'm trying to figure out how to get the same behavior as the current syslog state using your PR I fail to get a debug log to systemd but I don't get debug logs of the core/py3status if I try to use:

{
    "version": 1,
    "handlers": {
        "syslog": {
            "formatter": "default",
            "class": "logging.handlers.SysLogHandler"
        },
        "file": {
            "class": "logging.handlers.RotatingFileHandler",
            "filename": "/tmp/py3status_log.log",
            "maxBytes": 2048,
            "formatter": "default"
        }
    },
    "formatters": {
        "default": {
            "validate": false,
            "format":"%(asctime)s %(levelname)s %(module)s %(message)s",
            "datefmt":"%Y-%m-%d %H:%M:%S"
        }
    },
    "loggers": {
        "root": {"handlers": ["syslog"], "level": "DEBUG"},
        "core": {"handlers": ["syslog"], "level": "DEBUG"},
        "xrandr": {"handlers": ["file"], "level": "DEBUG"}
    }
}

what I am missing please?

ultrabug avatar Feb 01 '22 09:02 ultrabug

You were missing https://stackoverflow.com/a/3969772/340862: the syslog handler requires an "address" parameter (we are setting it correctly in the default setting in core.py.

By adding the same parameter to the json config, I am able to get the logs for core:

    "handlers": {
        "syslog": {
            "formatter": "default",
            "class": "logging.handlers.SysLogHandler",
            "address": "/dev/log"
        },
(...)

rlerm avatar Mar 10 '22 00:03 rlerm

This pull request introduces 1 alert when merging 004967107a72670e1759212c99c9a725dbc02be2 into afeab682409fafcb3948c2c1b6b63680b39f9e7c - view on LGTM.com

new alerts:

  • 1 for Variable defined multiple times

lgtm-com[bot] avatar Oct 02 '22 14:10 lgtm-com[bot]

closing, I never got to the bottom of it and I apologize for this

recent updates would require work from you

ultrabug avatar Jun 18 '23 09:06 ultrabug