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

advanced logging configuration #584

Merged
merged 26 commits into from
May 13, 2024
Merged

advanced logging configuration #584

merged 26 commits into from
May 13, 2024

Conversation

ekneg54
Copy link
Collaborator

@ekneg54 ekneg54 commented May 7, 2024

this PR aims to add more flexibility to the configuration of logpreps logging facility.
it implements the python dictConfig approach described here in the python logging documentation.

As we already use custom names for our loggers I decided to leave this contrary the best practice advice to use __name__ as the logger name. As a side effect we do not have a hierarchical order of our loggers anymore, but it gives us the flexibility to independently configure the log level of every logger in the configuration now. But I shortened the logger names in favor to get a better log output.

GOALS:

  • configurable logging format
  • add hostname to log entries
  • configurable date format
  • set level per log instance (root logger is DEBUG, but Pipeline 1 logs only ERROR)

TODO:

  • add tests
  • add documentation
  • add changelog
  • get rid of duplicate log messages

@ekneg54 ekneg54 added the enhancement New feature or request label May 7, 2024
@ekneg54 ekneg54 self-assigned this May 7, 2024
@ekneg54 ekneg54 force-pushed the dev-logging-dictconfig branch 2 times, most recently from ae503b3 to dc3b74c Compare May 10, 2024 13:31
@ekneg54 ekneg54 requested review from djkhl and dtrai2 May 11, 2024 12:02
@ekneg54 ekneg54 marked this pull request as ready for review May 11, 2024 12:03
@ekneg54 ekneg54 requested a review from ppcad May 11, 2024 12:03
Copy link
Contributor

@dtrai2 dtrai2 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I had a look at it and only found some minor suggestions. The first thing I noticed is though, that when running logprep with python logprep/run_logprep.py run ./quickstart/exampledata/config/pipeline.yml I get a stacktrace telling me that the FileNotFoundError ... multiprocdir .... Logprep should rather tell me in a propper logmessage that the multiprocdir is missing, at least the stacktrace shouldn't be visible.

logprep/runner.py Outdated Show resolved Hide resolved
logprep/util/configuration.py Outdated Show resolved Hide resolved
logprep/util/configuration.py Outdated Show resolved Hide resolved
logprep/util/logging.py Show resolved Hide resolved
tests/testdata/config/config.yml Show resolved Hide resolved
tests/unit/util/test_logging.py Outdated Show resolved Hide resolved
tests/unit/util/test_logging.py Outdated Show resolved Hide resolved
ekneg54 and others added 3 commits May 13, 2024 13:12
Co-authored-by: dtrai2 <95028228+dtrai2@users.noreply.github.com>
Co-authored-by: dtrai2 <95028228+dtrai2@users.noreply.github.com>
@ekneg54 ekneg54 requested review from dtrai2 and removed request for djkhl May 13, 2024 11:29
@ekneg54 ekneg54 merged commit 4508d24 into main May 13, 2024
10 checks passed
@ekneg54 ekneg54 deleted the dev-logging-dictconfig branch May 13, 2024 13:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants