uber-go / zap

Blazing fast, structured, leveled logging in Go.
https://pkg.go.dev/go.uber.org/zap
MIT License
22.08k stars 1.44k forks source link

Unexpected handling of variadic function arguments list in SugaredLogger #1449

Closed m0t9 closed 4 months ago

m0t9 commented 4 months ago

Describe the bug If one is trying to pass additional fields to SugaredLogger from variadic arguments of parental function, logger produces error "Ignored key without a value" together with proper fields handling message.

To Reproduce Code below allows to obtain described behavior with zap 1.27.0 and Go SDK 1.22.2

package main

import "go.uber.org/zap"

func addFieldsIteratively(logger *zap.SugaredLogger, fields ...string) *zap.SugaredLogger {
    // len(fields) - 1 --- in order to avoid possible dangling field
    for i := 0; i < len(fields)-len(fields)%2; i += 2 {
        logger = logger.With(fields[i], fields[i+1])
    }
    return logger
}

func addFields(logger *zap.SugaredLogger, fields ...string) *zap.SugaredLogger {
    // to get rid of possible dangling field
    validLen := len(fields) - len(fields)%2
    return logger.With(fields[:validLen])
}

func main() {
    l, _ := zap.NewProduction()
    s := l.Sugar()
    s = addFieldsIteratively(s, "key1", "value1", "key2", "value2") // as it expected to be
    s.Info("ok1")
    s = addFields(s, "key1", "value1", "key2", "value2")
    s.Info("ok2")
}

The output

{"level":"info","ts":1720006600.447523,"caller":"sandbox/main.go:23","msg":"ok1","key1":"value1","key2":"value2"}
{"level":"error","ts":1720006600.447597,"caller":"sandbox/main.go:16","msg":"Ignored key without a value.","key1":"value1","key2":"value2","ignored":["key1","value1","key2","value2"],"stacktrace":"main.addFields\n\t/Users/m.korinenko/GolandProjects/sandbox/main.go:16\nmain.main\n\t/Users/m.korinenko/GolandProjects/sandbox/main.go:24\nruntime.main\n\t/usr/local/opt/go/libexec/src/runtime/proc.go:271"}
{"level":"info","ts":1720006600.447663,"caller":"sandbox/main.go:25","msg":"ok2","key1":"value1","key2":"value2"}

First log message — as it expected to be, 2nd and 3rd — as it should not

Expected behavior The expected behavior is also demonstrated with function addFieldsIteratively

Additional context Seems like it handles as a regular slice, but at the same time produces expected log message

m0t9 commented 4 months ago

Okay, I always need to use here interace{} instead of string :)