Skip to content
This repository has been archived by the owner on Sep 14, 2020. It is now read-only.

Obey the configured levels when posting from logging #192

Merged
merged 4 commits into from
Sep 26, 2019

Conversation

nolar
Copy link
Contributor

@nolar nolar commented Sep 25, 2019

Fix an issue with k8s-events posted for log messages despite the configured levels.

Issue : fixes #188

Description

Configured log levels was ignored in the connector of the logging facilities to the k8s-event posting engine, so all events were posted. The docs were written for the event-posting shortcuts, but this was not taken into account when the log-posting engine was added.

In addition, some extra checks are added to verify that the shortcuts actually obey the configured levels.

Note that the debug level messages are now filtered not by the K8s-posting handler K8sPoster — the minimum level is removed from there— but by its filter() method, based on the Kopf's configs. Which is generally equivalent, as it was previously filtered by level in super().filter(record), which stands in the same line.

Types of Changes

  • Bug fix (non-breaking change which fixes an issue)

Review

List of tasks the reviewer must do to review the PR

  • Tests
  • Documentation

@nolar nolar requested a review from samurang87 as a code owner September 25, 2019 21:37
@zincr
Copy link

zincr bot commented Sep 25, 2019

🤖 zincr found 0 problems , 0 warnings

✅ Large Commits
✅ Approvals
✅ Specification
✅ Dependency Licensing

@@ -39,9 +40,10 @@ def createLock(self):
def filter(self, record):
# Only those which have a k8s object referred (see: `ObjectLogger`).
# Otherwise, we have nothing to post, and nothing to do.
fit_lvl = record.levelno >= config.EventsConfig.events_loglevel

Choose a reason for hiding this comment

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

fit?

Choose a reason for hiding this comment

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

maybe filter_level instead? :)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

"Fits the minimal level condition". Some help with proper wording is appreciated.

Choose a reason for hiding this comment

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

ooh

Choose a reason for hiding this comment

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

loglevel_higher? 🤔

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@samurang87 Fixed (probably).

@nolar nolar merged commit 5c11fa1 into zalando-incubator:master Sep 26, 2019
@nolar nolar deleted the loglevels-ignored branch September 26, 2019 10:56
@nolar nolar added the bug Something isn't working label Oct 22, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Setting kopf.EventsConfig.events_loglevel doesn't prevent Type=Normal Events to be posted
2 participants