Page MenuHomeSoftware Heritage

cli: redirect logging to user syslog when daemonized
ClosedPublic

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

Diff Detail

Repository
rDFUSE FUSE virtual file system
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

haltode created this revision.Thu, Oct 15, 10:32 AM

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.

olasd added a subscriber: olasd.Thu, Oct 15, 11:00 AM

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.

haltode updated this revision to Diff 15084.Thu, Oct 15, 12:05 PM

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.Thu, Oct 15, 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 a subscriber: zack.Thu, Oct 15, 12:47 PM
zack added inline comments.
swh/fuse/cli.py
114–119

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
114–119

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.

haltode updated this revision to Diff 15085.Thu, Oct 15, 1:13 PM

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.

olasd accepted this revision.Thu, Oct 15, 1:18 PM

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.Thu, Oct 15, 1:18 PM
haltode updated this revision to Diff 15096.Thu, Oct 15, 2:39 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.

haltode updated this revision to Diff 15144.Fri, Oct 16, 1:02 PM
  • 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.

haltode updated this revision to Diff 15159.Fri, Oct 16, 3:22 PM

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.

haltode updated this revision to Diff 15162.Fri, Oct 16, 3:34 PM

Rebasing on master.

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.