Ability to modify KV in Logger

[CodeSandwich]

Context

I have application divided into sections. Every section has own logger with its name in KV, so log consumer can tell which section is source of entry. I want to pass logger from a section to a sub-section and set a new section name in the KV to reflect that.

Problem

Simply pushing new KV with same key as existing one is wrong. Logger with Logger wrapped inside outputs entries from KVs from both of them. There is no overriding or shadowing. This causes problems with JSON output formats, which generate invalid structures with duplicate keys (I'm looking at you, slog-json)

I can't get rid of the old section name in sub-section either:

• Logger::new preserves whole old KV
• Logger can't have its drain extracted or KV dropped
• KV and OwnedKVList have no API for modification

Solutions

• Extend OwnedKVList API to have means of editing. Because of generic nature this will cause same change in KV, which in turn will make a huge breaking change in API.
• Add method to extract Drain or drop KV from Logger. That looks easiest, but is least powerful, allows only full reset of KV.
• Make Logger iterate over al KVs on all layers of wrapping and remove duplicates with preference of values less shallowly wrapped. This will solve JSON problem altogether.

[dpc]

Another solution is to implement a Drain that removes duplicates. This one is good because everyone can implement it on their own, and since they know which keys can change, they can optimize it a lot over a general solution.

It seems to me that it should be possible to have a function that takes old OwnedKVList and assembles a new, modified version. That can be passed to Logger::root to create a new Logger with replaced values, but same Drain. Maybe we should even add Logger::new_replace or something like this. I like this one.

[CodeSandwich]

Is it possible to create de-duplicating Drain with current API? AFAIK Drain::log receives &OwnedKVList, which can't do anything with its public API. This leads us back to my proposition 1, extending OwnedKVList's API.

[dpc]

@CodeSandwich Yes it is. You can serialize it into arbitrary Serializer https://docs.rs/slog/2.4.1/slog/trait.KV.html#tymethod.serialize and in that Serializer you can keep track of, skip, replace, and whatever you want with each key-value pair.

[CodeSandwich]

So if I understand correctly, the idea is to create Drain which on every log will create a fake serializer and pass it to KV. This serializer will intercept and store all keys and filter out duplicates based on their values. Finally Drain will extract stored mapping and pass it as &OwnedKVList down to its child.

The problem is that it's not possible to to create an OwnedKVList without macro and macro requires to know all keys during compilation.

This could be possible to use, but personally I would prefer to avoid it at any cost. The implementation looks moderately hard in single specific case, but it seems extremely hard to make configurable, probably possible only with some macro generating custom types. It also almost certainly involves allocation of collection on every log event.

[CodeSandwich]

I think that adding possibility to construct OwnedKV by hand could solve this issue. This is a simple newtype and a struct, so it would be 2 lines of code without risking breaking changes.

It would make it possible to create all kind of custom elements. I'm thinking about a custom DedupLogger which would store a HashMap of key-values along a copy of them in an OwnedKV. When sombody tries to wrap it in a new DedupLogger, they could agree on new set of key-values and construct new HashMap and OwnedKV to use by the parent. Because logging is always performed with macros, which are defacto duck-typed, DedupLogger could be used instead of Logger if only it had the same definition of log method. Not a very pretty solution, but definetely least effort on slog side.

[dpc]

We could add some constructors to OwnedKVList.

[CodeSandwich]

Why OwnedKVList and not OwnedKV? The latter is more generic and can generate OwnedKVList, but not the other way around.

[eranrund]

Making the Drain able to modify KV would also be useful to have a drain that includes the current thread name, which is a thing I've been trying to do unsuccessfully with the current API.

[dpc]

Just a thought - there's nothing preventing creation of a new Logger with a KV inside entirely rebuild. One could just iterate through all the KVs and clone them, but replace/insert new ones etc.

[dpc]

@eranrund What's the problem with current-thread? Is it async logger? Generally https://docs.rs/slog/2.5.1/slog/struct.FnValue.html should do, as long as async is not involved.

[eranrund]

@dpc It is async...

[dpc]

The missing piece is early evaluation. One approach would be to add a new field in the Record which would be "early-evaluated-owned-kvs". Then couple of functions to add them to Logger. slog-async would evaluate them early and pass further just like borrowed values. I think it would be a backward compatible change, and quite easy to make.

Eventually - it could be just added to slog-async itself. Just register KVs that Async will add. It makes sense to have it as a feature local to slog-async, as async is the only time this feature is going to be needed.

[eranrund]

I like that suggestion. Is that something you expect to have time and interest to build in the near future?

[dpc]

@eranrund slog is now entirely in "you fill PRs for what you need" mode. :D

[eranrund]

Fair! I'll look into doing that sometime soon, I hope. I'm in the process of converting a fairly large internal codebase to using slog, so I want to get us committed to using it and then I could implement this. Thank you!