Skip to content

Level/Context override storage #138

@nevans

Description

@nevans

I submitted a couple of PRs that solve the basic problems I have with the current level overrides:

But I'd prefer to change it a bit more.

If #132 (or something like it) is accepted:

  1. I believe that the "fiber scoped" block overrides for both #with_level and #with_context should use the same storage strategy. Rather than keeping two separate maps, I think it's better to combine them into a shared strategy: @override_store or @context_store.
  2. To share storage with a hash-like API, that suggests that level_key and context_key need to have different values. We could use something like Fiber[:logger_level_key] ||= Object.new, to support both Hash and ObjectSpace::WeakKeyMap, but that's kinda gross.

The current approach doesn't allow us to cleanly swap in the Fiber-storage API:

Fiber storage does face the additional question of how to handle when fibers aren't nested in a "structured concurrency" manner: does the child continue with its inherited storage after the parent leaves the #with_level or #with_context block. I think yes, it should... but others may want different behavior.

And many applications, frameworks, or libraries will already have their own encapsulation of execution context, which may not be identical to Fiber.current. As an example of this, see ActiveSupport::IsolatedExecutionState, which is used by the ActiveSupport::LoggerThreadSafeLevel mixin.

All these concerns can be addressed by creating a simple API, creating a default class that implements that API, and allowing an instance to be passed in as an argument to #initialize.

I propose something like the following:

class Logger
  class ContextStore
    def initialize
      @levels = ObjectSpace::WeakKeyMap.new
    end

    def level_override     = @levels[Fiber.current]
    def level_override=(val) @levels[Fiber.current] = val end

    def clear = @levels.clear

    private

    def initialize_copy(original)
      super
      @levels = @levels.dup
    end
  end

  def initialize(*, context_store: nil, **)
    # ...
    @context_store = context_store || ContextStore.new
    # ...
  end
end

This mimics the current implementation, with the addition of #136 and #137.

The benefits of this approach:

  • by encapsulating all execution state context, we can remove implementation details (like #level_key) from the Logger class.
  • it allows users to custom context storage without subclassing logger
  • it can easily be extended to use the same strategy for both level overrides and context

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions