Build is green

Patch application report for D7805 (id=28200)

Rebasing onto a3a2b11473...

Current branch diff-target is up to date.

Changes applied before test

commit d7591a6307471c6664b10334ceabd6c6b2effb4f
Author: Antoine R. Dumont (@ardumont) <ardumont@softwareheritage.org>
Date:   Tue May 10 18:02:12 2022 +0200

    Allow setting up logging level in core loader
    
    This will allow bumping the verbosity level when debugging some loaders. And actually
    lowering the current verbosity on existing loaders (e.g. cvs one) without losing the
    possibility to be more verbose in debugging mode.
    
    Related to T4214

See https://jenkins.softwareheritage.org/job/DLDBASE/job/tests-on-diff/785/ for more details.

Build is green

Patch application report for D7805 (id=28203)

Rebasing onto a3a2b11473...

Current branch diff-target is up to date.

Changes applied before test

commit 7fdd135b0f32d85a2e5ac459b57e1b7673d72921
Author: Antoine R. Dumont (@ardumont) <ardumont@softwareheritage.org>
Date:   Tue May 10 18:02:12 2022 +0200

    Allow setting up logging level in core loader
    
    This will allow bumping the verbosity level when debugging some loaders. And actually
    lowering the current verbosity on existing loaders (e.g. cvs one) without losing the
    possibility to be more verbose in debugging mode.

See https://jenkins.softwareheritage.org/job/DLDBASE/job/tests-on-diff/786/ for more details.

I'm a bit uncomfortable with adding more ad-hoc logger.setLevel calls deep in SWH code (which then make reasoning about the logging config harder), rather than ripping it all apart and properly setting up the logging machinery "the python way" with a clean, explicit dictConfig (potentially pulled from a yaml file set as an envvar?).

How does this interact with the swh.scheduler.celery_backend.config.setup_log_handler celery hook? I would expect that this will have no effect if the log handler (systemd or stdout) isn't set to allow DEBUG messages through.

Yeah, I'm not saying it should happen now, but the way this stuff has been handled has been inconsistent/painful/annoying me for a while, so maybe it's time?

What I would like™ to see happen:

• lift the logic of https://forge.softwareheritage.org/source/swh-core/browse/master/swh/core/cli/__init__.py$137-156 into a common utility function (probably moved to swh.core.logging/logger)
• light all of this https://forge.softwareheritage.org/source/swh-scheduler/browse/master/swh/scheduler/celery_backend/config.py$82-138 on fire and replace it with a call to the utility function instead, pulling its arguments from a couple of environment variables (and the celery command line if relevant). Ensure the envvars are consistent with the ones that the swh command line supports.
• replace this https://forge.softwareheritage.org/source/puppet-swh-gunicorn/browse/master/files/logconfig.ini and that https://forge.softwareheritage.org/source/puppet-swh-gunicorn/browse/master/templates/gunicorn-instance.cfg.erb$40-42 to use the same utility function, with consistent environment variables

and finally

• coalesce all the ad-hoc logger.setLevel calls strewn across our codebase, as well as gunicorn's hardcoded logconfig.ini, into a (bunch of) common yaml logging setup file(s) defined in puppet and docker
• update all the entry points / puppet deployment manifests / ... to use that common config file and envvar

In D7805#203180, @olasd wrote:

Yeah, I'm not saying it should happen now, but the way this stuff has been handled has been inconsistent/painful/annoying me for a while,

yeah, me too.

so maybe it's time?

I'd say at least it's time for filing a task about it with your plan below (thanks for detailing it even further).

What I would like™ to see happen:

• lift the logic of https://forge.softwareheritage.org/source/swh-core/browse/master/swh/core/cli/__init__.py$137-156 into a common utility function (probably moved to swh.core.logging/logger)
• light all of this https://forge.softwareheritage.org/source/swh-scheduler/browse/master/swh/scheduler/celery_backend/config.py$82-138 on fire and replace it with a call to the utility function instead, pulling its arguments from a couple of environment variables (and the celery command line if relevant). Ensure the envvars are consistent with the ones that the swh command line supports.
• replace this https://forge.softwareheritage.org/source/puppet-swh-gunicorn/browse/master/files/logconfig.ini and that https://forge.softwareheritage.org/source/puppet-swh-gunicorn/browse/master/templates/gunicorn-instance.cfg.erb$40-42 to use the same utility function, with consistent environment variables

and finally

• coalesce all the ad-hoc logger.setLevel calls strewn across our codebase, as well as gunicorn's hardcoded logconfig.ini, into a (bunch of) common yaml logging setup file(s) defined in puppet and docker
• update all the entry points / puppet deployment manifests / ... to use that common config file and envvar

Definitely a clear plan worth following.
I opened a task for it [1] (the title will need rework though)

[1] T4229