docs: add documentation on logging during HPI module development

[karlicoss]

@seanbreckenridge added some docs/guidelines on logging in modules, let me know if you think there is anything I missed, or some practices you prefer that could be useful!

[purarue]

Looks great :+1:

Probably not worth mentioning but the only other thing I commonly do is when writing other python libs/functions that would be used in a HPI module, is allow an optional logger to be passed so the library code respects HPI loglevel:

https://github.com/seanbreckenridge/HPI/blob/master/my/activitywatch/active_window.py#L45

https://github.com/seanbreckenridge/active_window/blob/545745581ed8c95f9b3cb8505c6de6d784d20908/active_window/parse.py#L48

thats more of just personal preference though, could always patch/override the loglevel based on the computed loglevel like is done in google takeout parser

probably a bit too involved to be mentioned in the docs as recommending either of these could be seen as a bit prescriptive and theres no real reason to specify one pattern here, but thought Id mention it anyways

[karlicoss]

Yeah, good point -- and thought about this too, the only reason I didn't mention this is because if the logger is reused in a library, it can be a bit confusing where the logging messages originate from (e.g. messages would appear under my.activitywatch even though the library is active_window), so I'm a bit undecided what's the best way to deal with this, so figured it's worth experimenting first. Maybe it should be a sublogger or something, e.g. partial(AW.parse_window_events, logger=logger.getChild('active_window'))? Still a little misleading but slightly less confusing.

[karlicoss]

Btw, also thinking it's probably worth switching LOGGING_LEVEL_module_name=<loglevel> to behave more like we did in cachew, i.e. using a glob https://github.com/karlicoss/cachew/commit/f71a505dcec07e2ad5b97c172882948b5f4429e9 That way it would be a bit mode intuitive, no need to replace dots in module names etc.

Also it would make LOGGING_LEVEL_HPI obsolete, could just pass my* as the glob.

The only difference is with cachew it was just on/off, whereas with logging you can have multple levels, so I guess the options are

• LOGGING_LEVEL_DEBUG="my.module1:my.module2*:..." LOGGING_LEVEL_ERROR="my.module3"
• LOGGING_LEVEL="my.module1=DEBUG:my.module2*=DEBUG:my.module3=ERROR:..."

The former looks a bit cleaner, but the latter can be more flexible, and the mechanism could be reused to control other aspects of HPI behaviour, so not sure -- what do you think?

[purarue]

I think it should probably have HPI in the name somewhere, so like HPI_LOGGING=..., like CACHEW_DISABLE

I think the second one is a bit nicer, and is generally how tools with modules like mpv does it: https://man.archlinux.org/man/mpv.1#msg

They use , instead of :, either probably works (could probably actually check for both, and warn the user, or just support both)

[purarue]

even though the library is active_window), so I'm a bit undecided what's the best way to deal with this, so figured it's worth experimenting first.

ah true. yeah, will experiment with this, not sure if theres one pattern thats best to recommend right now

[karlicoss]

I think it should probably have HPI in the name somewhere

Yeah, agree it would be more intuitive -- the main reason it's not like this is that I'm kinda dragging this logging helper around different projects (e.g. in promnesia/cachew/various DALs etc). So it's a bit easier/consistent if the env variable name is the same (considering the logging helper doesn't care where it's used from). So not sure what's the best way to deal with this..

[purarue]

hmm, right

obvious concern is that other applications may also use LOGGING_LEVEL and a user might need to set it to something else when using HPI which isnt the best experience...

maybe could just check both LOGGING_LEVEL and perhaps some other more-specific envvar? agree it isnt nice to hardcode an _HPI suffix if its used for other projects but should provide an escape hatch even if its slightly unintuitive incase the user is using LOGGING_LEVEL to configure something else

would probably check LOGGING_LEVEL_HPI (or some other hpi-namespaced) envvar first and then LOGGING_LEVEL, to prevent conflicts with other tools/applications

[karlicoss]

Ah yeah agree LOGGING_LEVEL is too generic. And yeah, adding HPI suffix/prefix could work, but perhaps we could come up with something less generic (but still generic enough). E.g. judging by https://github.com/search?q=PY_LOGGING_LEVEL&type=code , PY_LOGGING_LEVEL could work

[karlicoss]

@seanbreckenridge seems like my.browser.export has warnings level hardcoded, so end up a bit too quiet during processing exports incrementally. Do you remember why? I'd be keen to switch to default (info) if you don't have objections :) https://github.com/karlicoss/HPI/blob/5630621ec1be12144f6779f02984a79eacd90d24/my/browser/export.py#L28

[purarue]

no real reason, you can switch it to info if you'd like