Page MenuHomeSoftware Heritage

Allow setting up logging level in core loader
AbandonedPublic

Authored by ardumont on Tue, May 10, 6:03 PM.

Details

Summary

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 T4229

Diff Detail

Event Timeline

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.

Amend commit message (remove related to)

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.

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),

quite true, i just adapted this as it's our current (bad apparently) way of doing it.

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?).

Well, sure, that's more involved than i currently aimed at but sure.
So i'll check a bit later.

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.

lol, i've no idea.
Your expectations sounds reasonable to me indeed.

And after reading it thrice, i'm not sure i understood everything quite as you intended.
I'll check again tomorrow.

And thanks for the hints.

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:

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:

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

ardumont edited the summary of this revision. (Show Details)

Abandoned in favor of the future work in T4229