Logger local named tags [WIP]

[theocodes]

Issue #165

Changelog (TODO)

Pull requests will not be accepted without a description of this change under the [unreleased] section
in the file CHANGELOG.

Description of changes

Note: This isn't quite ready for prime yet but raising the draft early for some feedback and discussion.

I've taken an initial stab at this with the goal of adding the discussed functionality without breaking existing API or behavior. The proposed ideas seem to slot in nicely with current functionality in such a way that I wouldn't expect anything to break.

However, there's something I wanted to raise which I had initially taken for a given that actually turned out to be a little unintuitive, in my opinion.. This is very much an API design consideration, so your input would be valuable @reidmorrison.

Consider the following:

logger.named_tags = {test: "123"}

logger.tagged(important: true) do
  logger.info "test"

  some_other_logger.info "doing something"
end

As the PR currently is, "test" would contain both test and important while "doing something" would only contain important. This is aligned with what was discussed in the issue and it kind of makes sense. However, what got me thinking was the fact that we called #tagged on the logger instance here, which my intuition tells me would have an additive effect on the tags already set at the logger level. In other words, add to the logs within the block, any logs already present in the logger in addition to these i'm providing now.

This intuition comes from the fact that this is how things currently work today. Of course this comes from the fact that the only other tags that exist in addition to those passed to #tagged are those already set at the thread level. But if we are introducing this "third layer", we need to decide on which makes more sense.

As it currently stands, I don't know if theres much practical difference in using logger.tagged vs SemanticLogger.tagged, so I think we have two options if tagged is called on the logger:

• Allow logger-local tags to be inserted into all entries within tagged, regardless of the receiver.
• Update the logic to only include tagged tags to the logger's own logs.

I think either of the two would help in keeping things consistent, though the second option would obviously be a bit more work and would definitely break current behavior.

Frankly I don't know which is better and like I mention, this is more of a design consideration. Personally I feel like the first option is the way to go, but there's a very possible chance I'm missing part of the puzzle here, so any input would be appreciated.

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

[theocodes]

Not sure this was the best way to handle this. Thought of maybe passing the tags to the log initializer but was conscious that might break the API. 🤔

[theocodes]

Slightly unrelated - I noticed the filter spec on test/logger_test.rb is only testing against :debug and has a line commented out. Test seem to pass with the commented out line - is there a reason it's commented out?

[theocodes]

Is it just me or this feels a little strange?
I'm thinking the expected behavior would be for the whole value to be overridden, right?