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

Pquisby logging (0.0.27) is problematic #25

Open
dbutenhof opened this issue Oct 14, 2023 · 1 comment · May be fixed by #32
Open

Pquisby logging (0.0.27) is problematic #25

dbutenhof opened this issue Oct 14, 2023 · 1 comment · May be fixed by #32
Assignees

Comments

@dbutenhof
Copy link
Collaborator

With Pquisby 0.0.27 we've noticed two changes:

  1. Creating a ~/.pquisby/pquisby.log file (of which we'd been unaware until we got a failure creating it during containerized testing, though this doesn't happen usually).
  2. We direct our logs through rsyslogd to the RDU2 OpenSearch dashboard for review, and this week we're seeing doubled log entries, one with our normal format and one with a simplified format.

Looking at the (installed) 0.0.27 code, I can see that

  1. logging_configure.configure_logging() is called unconditionally when post_processing.py is imported;
  2. The logging configure sets up both a RotatingFileHandler to ${HOME}/.pquisby/pquisby.log and a StreamHandler with default arguments, which will log to stderr, and installs both on the process root logger with logging.basicConfig.

While the file handler isn't a problem on its own, I think you really should be doing this on your own Pquisby logger object, not on the global root logger which affects every (Python) logger in the process. Copying Pbench logs into ~/.pquisby/pquisby.log doesn't help anyone! And also logging to stderr means that, in a containerized environment, every log is being written three times: to syslog, to your rotating log file, and to stderr. And, in particular, this accounts for the doubled logs we're seeing on OpenSearch, which is highly undesirable.

Please don't do this! Pquisby can log its own internal messages to a file and/or to stderr if desired, but don't modify the process root logger, which affects everyone else in the process as well. Ideally, something like logging.getLogger("pquisby") at runtime (rather than import time), and add your handler(s) to that rather than to the root logger.

@sousinha97
Copy link
Collaborator

sousinha97 commented Oct 16, 2023 via email

@sousinha97 sousinha97 self-assigned this Oct 26, 2023
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 a pull request may close this issue.

2 participants