Methods to Interact With Sink Attributes

[michohl]

I'm working on a Kubernetes controller and in my Reconcile loop I want to dynamically set some fields using information that can't be determined until I'm inside the Reconcile loop.

The reason this is a problem is because this function is sometimes called recursively. This means if I use either of the following:

r.Log = r.Log.WithName("something")

or

r.Log = r.Log.WithValues("key", "value")

Then I will end up with log output where the name is "somethingsomethingsomething" or I have duplicates of the key key with different values which makes the field useless.

Would it be possible to implement a way for the user to do the following actions?

1. Check the current value of an attribute in the logger
   • This would allow me to check "Did I set the name of my logger already?" so I can avoid calling WithName multiple times
2. Delete attributes that were previously assigned
   • If not then maybe some way of setting the logger to replace keys if a duplicate value is specified?

Example of how I'm imagining these proposed methods would look in practice:

// Set the name only if the current logger's name is not set
if r.Log.GetSink().GetName() == "" {
    r.Log = r.Log.WithName("something")
}

// Set value for `key` without duplicating
if r.Log.GetSink().GetValue("key") != "" {
    r.Log = r.Log.GetSink().DeleteAttribute("key")
}
r.Log = r.Log.WithValues("key", "value")

[thockin]

At best, these would be optional interfaces, so you'd have to do something like:

func myFunc(log logr.Logger) {
    if name, ok := log.GetName(); ok {
        if name != "something" {
            log = log.WithName("something")
        }
    }
}

Underneath, it would test the LogSink for support and tell you if it supported it or not.

Deleting a key would be the same, though I am more suspicious of that use-case.

A more DIY approach would be something like:

func MyFunc(log logr.Logger) {
    myFuncInternal(logr.WithName("something").WithValues("key", "value"))
}

func myFuncInternal(log logr.Logger) {
    // do stuff
    // recursion calls myFuncInternal
}

[pohly]

I have duplicates of the key key with different values which makes the field useless.

I'm not so sure about this. My understanding of the "repeated key" semantic is that the last valye wins, so (in JSON) {"foo": "bar", "foo": "x"} would be equivalent to {"foo": "x"}. But later I learned that the JSON spec doesn't actually guarantee that. It's up to the parser which value it uses.

This makes it a problem of the log backend. The klog text backend doesn't de-duplicate logger.WithValues("foo", "var").WithValues("foo", "x"). It does de-duplicate logger.WithValues("foo", "bar").Info("something", "foo", "x"). My rationale was that WithValues is easier to keep under control such that keys are unique, whereas duplication of keys with log calls is currently unavoidable in Kubernetes (contextual logging and thus WithValues is optional). Given what you described it would make more sense to always de-duplicate everything, even if that causes overhead.

zap doesn't de-duplicate. It seems to rely on JSON parsers using the latest value.

[thockin]

Strictly, JSON fields are reorderable. Practically, I think plenty of things depen on in-order processing.

FWIW: funcr does not de-dup keys, either. It could de-dup across successive WithValues calls, but de-duping across WithValues and Info/Error calls is harder and more expensive (WithValues is pre-rendered).