Project

General

Profile

Actions

Refactor #22354

closed

Make SilencedLogger thread safe

Added by Lukas Zapletal about 6 years ago. Updated almost 6 years ago.

Status:
Closed
Priority:
Normal
Category:
Logging
Target version:
Fixed in Releases:
Found in Releases:

Description

Rails logger silencer code was thread safe for some time:

https://github.com/rails/rails/commit/629efb605728b31ad9644f6f0acaf3760b641a29

But this has been refactored in Rails 5.0 and thread safety is now part of logger instead of silencer:

https://github.com/rails/rails/commit/2518bda97cbbcb33dc9a92e70d5b01c09e64d12d

The patch made assumption that all Rails applications are using Rails logging stack, but that's not the case for Foreman - we rely on much more flexible "logging" gem (https://github.com/TwP/logging) which does not implement this type of thread safety for levels.

Foreman has its own logging delegator called SilencedLogger and the code there was never thread safe, concurrent sprockets requests are now using silence method which is not thread safe at all, with logging gem this leads to level set to ERROR (the default) after few requests in development mode which renders Foreman unusable (no debug logging output). This patch uses the same technique and stores temporary levels into thread-only context, so on concurrent access there are no incorrect readouts.


Related issues 1 (0 open1 closed)

Is duplicate of Foreman - Bug #21870: app logger dies when an exception is thrownDuplicate12/05/2017Actions
Actions #1

Updated by The Foreman Bot about 6 years ago

  • Status changed from New to Ready For Testing
  • Pull request https://github.com/theforeman/foreman/pull/5197 added
Actions #2

Updated by Lukas Zapletal about 6 years ago

  • translation missing: en.field_release set to 296
Actions #3

Updated by Lukas Zapletal about 6 years ago

  • Status changed from Ready For Testing to Closed
  • % Done changed from 0 to 100
Actions #4

Updated by The Foreman Bot about 6 years ago

  • Pull request https://github.com/theforeman/foreman/pull/5252 added
Actions #5

Updated by The Foreman Bot about 6 years ago

  • Pull request https://github.com/theforeman/foreman/pull/5260 added
Actions #6

Updated by Tomer Brisker about 6 years ago

  • Is duplicate of Bug #21870: app logger dies when an exception is thrown added
Actions

Also available in: Atom PDF