Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Unify logging with QueueHandler #1292

Merged
merged 4 commits into from
Sep 1, 2021

Conversation

TheTechromancer
Copy link
Contributor

@TheTechromancer TheTechromancer commented Aug 23, 2021

This PR creates a logging handler for SQLite and consolidates all logging activity into its own thread. This is accomplished with Python's built-in classes logging.handlers.QueueHandler and logging.handlers.QueueListener.

Provided it is properly initialized, logging can now be performed anywhere without worrying about database locks, processes, or threads:

import logging

log = logging.getLogger(f"spiderfoot.{__name__}")
log.info("test")

This PR is adapted directly from the official multiprocessing example in Python Logging Cookbook.

Logging code resides in a new file, spiderfoot/logger.py, which exposes functions for initializing logging in SpiderFoot.

When the main process starts, logging is initialized like so. Note that queue.Queue() can be used instead of multiprocessing.Queue() if multiprocessing is not in use.

import logging
from multiprocessing import Queue
from spiderfoot.logger import logListenerSetup, logWorkerSetup

loggingQueue = Queue()
logListenerSetup(loggingQueue, sfConfig)
logWorkerSetup(loggingQueue)

log = logging.getLogger(f"spiderfoot.{__name__}")
log.info("test")

logWorkerSetup is called once for each new multiprocessing.Process() that is spawned. Note that the loggingQueue variable is passed into the new process.

import logging
from spiderfoot.logger import logWorkerSetup

logWorkerSetup(loggingQueue)

log = logging.getLogger(f"spiderfoot.{__name__}")
log.info("test")

Functionally, the logging logic remains untouched except for one change. The SpiderFoot "root logger" has been changed from logging.getLogger() to logging.getLogger("spiderfoot"). This was done to avoid unintentionally scooping up log messages from third-party libraries or clobbering the log settings of automation scripts that call SpiderFoot functions. If this is not a desired change, let me know and I can switch it back.

@codecov-commenter
Copy link

codecov-commenter commented Aug 23, 2021

Codecov Report

Merging #1292 (c771be0) into master (08bfc2d) will increase coverage by 0.66%.
The diff coverage is 90.62%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #1292      +/-   ##
==========================================
+ Coverage   52.52%   53.19%   +0.66%     
==========================================
  Files         472      469       -3     
  Lines       38702    38133     -569     
==========================================
- Hits        20330    20283      -47     
+ Misses      18372    17850     -522     
Impacted Files Coverage Δ
test/unit/test_spiderfoot.py 97.73% <ø> (-0.02%) ⬇️
sfwebui.py 61.68% <77.77%> (+0.20%) ⬆️
sf.py 53.86% <83.33%> (-1.85%) ⬇️
spiderfoot/logger.py 92.18% <92.18%> (ø)
sflib.py 72.39% <100.00%> (-0.29%) ⬇️
sfscan.py 82.90% <100.00%> (+0.18%) ⬆️
spiderfoot/plugin.py 87.58% <100.00%> (ø)
spiderfoot/db.py 77.21% <0.00%> (-0.53%) ⬇️
modules/sfp_totalhash.py
test/unit/modules/test_sfp_totalhash.py
... and 14 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 08bfc2d...c771be0. Read the comment docs.

@TheTechromancer TheTechromancer marked this pull request as draft August 25, 2021 15:16
@TheTechromancer TheTechromancer marked this pull request as ready for review September 1, 2021 15:12
@smicallef smicallef merged commit 11fff7d into smicallef:master Sep 1, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants