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

Added support for structured logs #333

Merged
merged 4 commits into from
Sep 1, 2023

Conversation

mambelli
Copy link
Contributor

Added changes to support structured logs:

  • using structlog https://www.structlog.org/en/stable/
  • added supporting functions and structlog configuration in logSupport
  • consolidated log and handlers configuration and set up
  • updated all files using the framework loggers
  • added configuration knobs
  • added documentation

Structured logs are disabled by default to ease the transition. They can be enabled by adding structured="True" to the configuration of the logs (process_log sections in the Factory and Frontend configurations)

Added also some general improvements: cleanup of comments and docstrings, consolidation of util.is_true(), and fixed a couple of small bugs.

The first commits are by @MissAnita2

This PR replaces PR #327 because GitHub got confused from the rebase eliminating the HTML files changes.
And that PR replaced Anita's PR #325 because I reverted once the automatic unrelated changes to all HTML files and was unable to force-update when they were re-applied

Suggestions from @namrathaurs review of PR #327 have been applied

factory/glideFactoryEntryGroup.py Outdated Show resolved Hide resolved
factory/glideFactoryEntryGroup.py Outdated Show resolved Hide resolved
frontend/glideinFrontendElement.py Show resolved Hide resolved
unittests/worker_scripts/log_writer.py Outdated Show resolved Hide resolved
unittests/worker_scripts/log_writer.py Outdated Show resolved Hide resolved
unittests/test_lib_logSupport.py Outdated Show resolved Hide resolved
lib/logSupport.py Outdated Show resolved Hide resolved
@namrathaurs
Copy link
Contributor

Minor changes suggested.

Note: The failing pylint build check was discussed as a known problem, but based on Marco's understanding, this is expected to be resolved once the changes get merged with the main branch.

Copy link
Contributor

@namrathaurs namrathaurs left a comment

Choose a reason for hiding this comment

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

Left a comment regarding the obsolete PR reference in the CHANGELOG. Otherwise, everything looks good to be merged.

frontend/glideinFrontendElement.py Show resolved Hide resolved
factory/glideFactoryEntryGroup.py Outdated Show resolved Hide resolved
CHANGELOG.md Outdated Show resolved Hide resolved
frontend/glideinFrontendElement.py Show resolved Hide resolved
factory/glideFactoryEntryGroup.py Outdated Show resolved Hide resolved
@mambelli mambelli force-pushed the anita-structLogChanges_v2 branch 4 times, most recently from 3fc6667 to ccb8b50 Compare August 26, 2023 06:58
@mambelli mambelli force-pushed the anita-structLogChanges_v2 branch 3 times, most recently from 503084a to 11019dd Compare August 31, 2023 17:30
…ct fields/keys and improved log initialization and consolidated handler's setup

Using booleans True/False instead of 1/0 in rollover control in logSupport
Reverted manual_glidein_submit.py to use std Python logging (it is and standalone script not using the framework log files)
Fixed type of log-rotation specifications (maxDays, minDays and maxMBytes)  that can be float
Fixed minDays that previously was ignored
Added documentation and adapted unit tests
Added unit test for logging level
Consolidated also is_true() to lib/util.py
Cleaned up some docstrings and comments
@mambelli
Copy link
Contributor Author

Locally is completing successfully all unit tests. Before the time rotation was timing out. The culprit was the maxDays value converted to int. The specs in the class were not clear. Fractions of a day (float) are OK and used to run the unit test in a reasonable time.

Copy link
Contributor

@namrathaurs namrathaurs left a comment

Choose a reason for hiding this comment

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

Changes look good and are ready to be merged.

@mambelli mambelli merged commit 56cddc8 into glideinWMS:master Sep 1, 2023
9 checks passed
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.

3 participants