negative V value gets set to 0 which limits slog level usage

[logikone]

docs state:

logr/README.md

Lines 196 to 197 in a64db0b

	- All logr verbosity levels can be mapped 1:1 to their corresponding slog level
	by negating them.

However, when passing a negative int to V it gets set to 0:

logr/logr.go

Lines 308 to 317 in a64db0b

	func (l Logger) V(level int) Logger {
	if l.sink == nil {
	return l
	}
	if level < 0 {
	level = 0
	}
	l.level += level
	return l
	}

This makes slog.LevelWarn inaccessible.

[thockin]

I don't recall why the "all levels can be mapped" language was added. The reason we don't support negative V levels is because we didn't want code whose logger had been "pre-biased" to "bypass" the bias. @pohly somewhat disagrees and we have not had a real reason to resolve it.

See also #258

Also, there's a philosophical mismatch here - logr holds as truth that "warning logs" are not a real thing. There are things worth logging and things not worth logging. Deciding at the logging call-site whether a log-line is "significant" assumes that you know that a priori. My own experience says otherwise, and the use of "warn" almost always becomes muddy over time.

@pohly - thoughts?

[pohly]

The docs say "All logr verbosity levels can be mapped" - note that it does not say that all slog levels can be mapped.

As @thockin said, by design logr only supports a half-open range of verbosity levels (>= 0) whereas slog uses a range that is open both ways (negative and positive).

we didn't want code whose logger had been "pre-biased" to "bypass" the bias

Code can already be abusive by using logger.V(0).Info for debug messages or worse, logger.Error for harmless stuff. Not allowing V(-1) helps by ensuring that code cannot raise its importance past what the caller chose for it (e.g. logger.V(5)), but that doesn't work for logger.Error. Ultimately, the code has to be trusted to use logging responsibly.

@pohly somewhat disagrees and we have not had a real reason to resolve it.

My use case still stands: if we allowed V(-1), the caller of some code could make that code more verbose by passing logger.V(-1) instead of logger.

[logikone]

I'm attempting to use logr with a slog backend so we can use the same logging API for Kubernetes operators using controller-runtime as well as other parts of our application that run on Kubernetes but are not operators. There are times where things happen that are more than informational but do not really require an error message being logged. I suppose I could just log them at an info level with an extra key, I just figured it would be easier to set the V level to match the slog level.

[logikone]

Maybe we could map the V levels differently in the slog sink? For Example:

func (l *slogSink) Info(level int, msg string, kvList ...interface{}) {
	var slogLevel slog.Level

	switch {
	case level == 0:
		slogLevel = slog.LevelInfo
	case level == 1:
		slogLevel = slog.LevelWarn
	case level >= 2:
		slogLevel = slog.LevelDebug
	}

	l.log(nil, msg, slogLevel, kvList...)
}

Right this is how I'm setting up our logger:

logr.FromSlogHandler(slog.NewTextHandler(os.Stdout, &slog.HandlerOptions{
	Level: cfg.LogLevel,
}))

With this if slog.HandlerOptions.Level is set to slog.LevelInfo I get V(0) logs, if its set to slog.LevelWarn I get nothing, and if set to slog.LevelDebug I get everything. Log lines are printed with levels like:

level=INFO msg=v(0)
level=DEBUG+3 msg=v(1)
level=DEBUG+2 msg=v(2)
level=DEBUG+1 msg=v(3)
level=DEBUG msg=v(4)

Which does allow me to distinguish between different levels but isn't really intuitive.

[pohly]

When using the logr API, logger.V(0).Info == logger.Info is considered more important than logger.V(1).Info. With slog, slog.LevelInfo is less important than slog.LevelWarn. Therefore mapping V(1) to slog.levelWarn doesn't make sense to me.

Can we go back and look at what you are trying to achieve? If I get it right, you have code which uses the logr API and thus log levels >= 0. Do you also have code which uses the slog API?

You now want to emit log entries which are "more important" than V(0), for example logger.V(-4) (slog.LevelWarn == 4). Why is that useful?

In Kubernetes, we start with 0 = "most important, always enabled". We use logger.Error for "must be looked at by admin". Everything else is less important. I don't see a need for a "warning" level.

[thockin]

Exactly. We could map V(0) to slog Warn, V(1) to warn-1, ... V(4) to slog Info, ... V(8) to slog Debug.

But then the default level for a longer.Info() would be Warning. That doesn't seem right.

Life embraces the idea that there's no such thing as a warning. Just Info and Error. If anything, logger.Info() should have been logger.Log()