Page MenuHomeSoftware Heritage

cli: redirect logging to user syslog when daemonized
ClosedPublic

Authored by haltode on Oct 15 2020, 10:32 AM.

Diff Detail

Repository
rDFUSE FUSE virtual file system
Branch
feature/logging-files-preserve
Lint
No Linters Available
Unit
No Unit Test Coverage
Build Status
Buildable 16372
Build 25213: Phabricator diff pipeline on jenkinsJenkins console · Jenkins
Build 25212: arc lint + arc unit

Event Timeline

Build is green

Patch application report for D4265 (id=15067)

Rebasing onto 11ef971ac1...

Current branch diff-target is up to date.
Changes applied before test
commit c0f27fbbde0da994c18bef7af88ea68ede2ae85d
Author: Thibault Allançon <haltode@gmail.com>
Date:   Wed Oct 14 14:48:50 2020 +0200

    cli: do not close logging's file handlers when daemonized

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

I'm not sure this would work if someone attached a handler to a separate logger (e.g. if you had set up logging for swh.fuse separately from the root logger). This also won't work if the logger doesn't have the stream attribute (f.e. SyslogHandler has a socket attribute instead).

I think the most generic solution would be initializing logging again in the daemonized subprocess:

  • before daemonizing, call logging.shutdown() so the subprocess will not inherit any log handlers (with potentially closed handles)
  • after daemonizing, re-initialize logging

As a first approximation until the proper utilities for logging initialization are available in swh.core, I suggest that you just do a logging.BasicConfig(..., handlers=SyslogHandler(...)) in the daemonized process.

Init logging inside daemonized process

haltode retitled this revision from cli: do not close logging's file handlers when daemonized to cli: redirect logging to user syslog when daemonized.Oct 15 2020, 12:05 PM

Build is green

Patch application report for D4265 (id=15084)

Rebasing onto e8307fe54d...

First, rewinding head to replay your work on top of it...
Applying: cli: redirect logging to user syslog when daemonized
Changes applied before test
commit f63e33014b087ac200e6c4eccfc6b11836c12929
Author: Thibault Allançon <haltode@gmail.com>
Date:   Wed Oct 14 14:48:50 2020 +0200

    cli: redirect logging to user syslog when daemonized

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

zack added inline comments.
swh/fuse/cli.py
118–123

I don't get this. Aren't you here just throwing away the logging config that you had initially inherited from swh.core.cli?

AFAICT what you should do instead is re-enact the same configuration after daemonizing. Otherwise how could the user define a logging configuration at all?

Meh.

I would have expected logging.shutdown to properly tear down the config, and deallocate all loggers (so basicConfig can do its work again without interference). Looks like I was wrong: it only *closes* all handlers, but it keeps the full hierarchy of loggers with handlers (that are now closed!).

Looks like the proper way of resetting the logging config to a fully known state is to use the logging.config.dictConfig function: this erases the whole logging configuration and replaces it with the one passed in the dict. Something like:

logging.config.dictConfig({
    'version': 1,
    'handlers': {
        'syslog': {
            'class': 'logging.handlers.SyslogHandler',
            'address': '/dev/log',
        },
    },
    'root': {
        'handlers': ['syslog'],
    },
})
swh/fuse/cli.py
118–123

For now, yes.

Once swh.core.logger has a function to "reenact the same configuration" (which isn't currently possible, as we're calling the logging setup directly in the swh entry point, and "losing" the config information), then this should be using that.

Use dictConfig instead of basicConfig

Build is green

Patch application report for D4265 (id=15085)

Rebasing onto e8307fe54d...

First, rewinding head to replay your work on top of it...
Applying: cli: redirect logging to user syslog when daemonized
Changes applied before test
commit 75a5f891e765ed20134d903af4c45d426cca3597
Author: Thibault Allançon <haltode@gmail.com>
Date:   Wed Oct 14 14:48:50 2020 +0200

    cli: redirect logging to user syslog when daemonized

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

The logic looks fine to me. You may want to recover the current loglevel from the root logger instead of clamping it to INFO (which would supposedly make debugging the daemonized process easier?).

Now the question whether the needed utilities in swh.core should be implemented now rather than working around their limitations stays open. I'll tentatively accept this diff as it's definitely better than the status quo, even if not perfect.

This revision is now accepted and ready to land.Oct 15 2020, 1:18 PM

Use ctx.obj["log_level"] instead of hard-coded INFO

Build is green

Patch application report for D4265 (id=15096)

Rebasing onto e8307fe54d...

First, rewinding head to replay your work on top of it...
Applying: cli: redirect logging to user syslog when daemonized
Changes applied before test
commit 5d75e98d9e1ce06474b9debdc912b0fac21b29d9
Author: Thibault Allançon <haltode@gmail.com>
Date:   Wed Oct 14 14:48:50 2020 +0200

    cli: redirect logging to user syslog when daemonized

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

  • Rebase on master
  • fuse: add error logging in main

Build is green

Patch application report for D4265 (id=15144)

Rebasing onto bbcd975f8c...

Current branch diff-target is up to date.
Changes applied before test
commit 66a1d8fe16f89d54ed11056d8ef587ae2265d6f2
Author: Thibault Allançon <haltode@gmail.com>
Date:   Fri Oct 16 12:59:01 2020 +0200

    fuse: add error logging in main

commit 5b33ce39d71dd2cf1d1f43b27eca558fe2c51472
Author: Thibault Allançon <haltode@gmail.com>
Date:   Wed Oct 14 14:48:50 2020 +0200

    cli: redirect logging to user syslog when daemonized

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

Move pyfuse3 init inside try/catch

Build is green

Patch application report for D4265 (id=15159)

Rebasing onto 490c0de038...

First, rewinding head to replay your work on top of it...
Applying: cli: redirect logging to user syslog when daemonized
Applying: fuse: add error logging in main
Changes applied before test
commit a0e401c6b2879599c23dfa8c7afd3ac152ec408e
Author: Thibault Allançon <haltode@gmail.com>
Date:   Fri Oct 16 12:59:01 2020 +0200

    fuse: add error logging in main

commit 1ba946b89dd7a701aa087d32fa65615139bf5f3e
Author: Thibault Allançon <haltode@gmail.com>
Date:   Wed Oct 14 14:48:50 2020 +0200

    cli: redirect logging to user syslog when daemonized

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

Build is green

Patch application report for D4265 (id=15162)

Rebasing onto 6cf27dddec...

Current branch diff-target is up to date.
Changes applied before test
commit a8af4ad5a8bcc2dd17949e4278ce9ceb760b684c
Author: Thibault Allançon <haltode@gmail.com>
Date:   Fri Oct 16 12:59:01 2020 +0200

    fuse: add error logging in main

commit 68c9dcb81572d4ff23320edac0d51a1f8dd95049
Author: Thibault Allançon <haltode@gmail.com>
Date:   Wed Oct 14 14:48:50 2020 +0200

    cli: redirect logging to user syslog when daemonized

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