log/alt_experimental_level: concept demonstration

[seh]

As a foil to #357, demonstrate an alternate approach to the "level" package offered via the "github.com/go-kit/kit/log/experimental_package" import path, allowing use of either a global log event level filtering threshold or local filtering via instances of the level.Leveler interface.

Features not accommodated here for now:

• Returning an error from log.Log when squelching an event
• Returning an error from log.Log for events that lack a level
  (This feature doesn't make much sense with this approach.)
• Squelching events that lack a level
  (This feature doesn't make much sense with this approach.)
• Concurrent-safe adjustment of the global level threshold
  (Note the "alternately" comments in file global.go for one proposed solution.)
• Documentation
  (We could adapt much of this from the sibling package.)

See the commit message for then benchstat comparison.

This is a proof of concept. If others like this approach, we could refine whether to handle global configuration changes more safely (whether via atomic.Value, or guarding it with a sync.RWMutex), and whether to accommodate returning an error from log.Log, which wouldn't be too hard, whether via a singular sentinel error instance or with a caller-supplied error instance captured at configuration time.

Accommodating adjustment of the global level threshold safely imposes a performance tax on every subsequent event-recording call. I'd prefer to just document that callers should only change the threshold in an init function.

[seh]

Also worth considering: functions to check whether a given level is currently allowed. This allows callers to decide whether to avoid running some costly operation to produce log values if the event won't be recorded anyway.

These would look something like the following, both on the level.Leveler interface and available globally (though those are vulnerable to check-then-act mistakes):

• DebugIsAllowed() bool
• InfoIsAllowed() bool
• WarnIsAllowed() bool
• ErrorIsAllowed() bool

That's eight more identifiers that we'd export from the package.

Another idea—though easy enough for callers to implement—is a narrowing Leveler decorator that takes a Leveler and restricts it further, if necessary. That is, something like a family of these:

• AllowingErrorOnly(Leveler) Leveler

or, turning it around,

• SquelchingWarnAndBelow(Leveler) Leveler

[ChrisHines]

@seh Thanks for submitting your ideas. This way of handling leveled logging is certainly an interesting alternative to other approaches we've tried.

I have similar concerns with this variation as with the experiment in #357. Specifically the use of global state, although convenient, seems to almost always end with regret in the long run for widely reusable components.

[seh]

@ChrisHines Using the global functions here is optional, much like the flag package. It's a convenience, but if you can't afford that convenience, you're left with a second stateful thing to carry around (here, a Leveler), as opposed to hiding that state in a Logger like the "experimental_level" one does. In my applications I'd use that global access option.

@peterbourgon, I'd like to hear your thoughts as well. Is it worth proceeding with documentation?

[peterbourgon]

Unfortunately, I am pretty categorically opposed to package-global state.

[peterbourgon]

What's the chief difference or advantage to this approach over the existing one in your view, @seh? Performance, friendlier API, both, something else?

[seh]

The main advantage is CPU efficiency, avoiding inspecting log records to figure out whether their level is allowed to pass through, and avoiding collecting of records (their key/value pairs) that will get filtered due to their level.

Providing the global functions turned out to be a distraction here. I have (yet) another approach that I'd like to try, which will fall somewhere in between what I proposed here (minus the global functions) and your "experimental_level" package. I was out on vacation for a week, but now that I'm back I'll endeavor to flesh out the idea this week.

[peterbourgon]

Looking forward to it.

[peterbourgon]

@seh So I'd like to cut a Go kit 1.0.0 in the next few days, and I'd like for the current package experimental_level to be promoted to package level. I've been using it in anger on a couple of projects, and I'm happy with the experience. But if you have something cooking, I'd like to give that proper consideration first. Any update?

Also, @ChrisHines, would you have any objection to the above?

[ChrisHines]

@peterbourgon I don't object to promoting experimental_level. I do suggest that you create an issue or milestone for doing a 1.0.0. It would be good to give contributors a chance to coordinate with you and prompt people like me to submit any work they have been procrastinating.

[seh]

Please see a related proposal in #444, which removes the global state and embeds the level threshold in a logger.Logger instance, rather than in a companion level.Leveler instance as we did here.