DefaultLogger.WithCaller() is broken with slog

[jb-abbadie]

Description

The WithCaller() option of the pterm logger is broken when used with pterm.NewSlogHandler and always returns /slog_handler.go:55 as the caller

How to reproduce

package main

import (
        "log/slog"

        "github.com/pterm/pterm"
)

func main() {
        pterm.DefaultLogger.WithCaller().Info("default logger")

        logger := slog.New(pterm.NewSlogHandler(pterm.DefaultLogger.WithCaller()))
        logger.Info("slog logger")
}

expected result

2023-12-28 15:56:32 INFO  default logger caller: /tmp/slog_pterm/main.go:10
2023-12-28 15:56:32 INFO  slog logger caller: /tmp/slog_pterm/main.go:13

actual result

2023-12-28 15:56:32 INFO  default logger caller: /tmp/slog_pterm/main.go:10
2023-12-28 15:56:32 INFO  slog logger caller: /slog_handler.go:55

Impacted version

Tested with pterm v0.12.72 and Go 1.21.5

[ccremer]

You most likely will have to set the caller offset as well.
In my case I had to set it to 4 but depending on the call stack may need a different value. The higher value, the higher it goes back in the call stack. Try around a bit

	WithCaller().WithCallerOffset(4)

[MarvinJWendt]

As @ccremer said, you would need to set an offset here. For your specific example, an offset of 3 works:

pterm.DefaultLogger.WithCaller().Info("default logger")

logger := slog.New(pterm.NewSlogHandler(pterm.DefaultLogger.WithCaller().WithCallerOffset(3)))
logger.Info("slog logger")

We could try to guess the offset, if pterm.NewSlogHandler is used, and the passed Logger has no specified offset, but that would most likely be a wild and nondeterministic guess, as the offset could change between Go versions.

I will definitely document this on the WithCaller method.

[jb-abbadie]

Thanks everybody for your quick response. WithCallerOffset indeed fixes the issue.

[ccremer]

@MarvinJWendt If we are talking breaking changes, maybe WithCaller could also receive an integer with the offset? That way, since the offset is probably required anyway to get it working correctly, people would be less forgetful about this.
E.g. WithCaller(3), or WithCaller(true, 3)
Or alternatively, remove WithCaller completely and set the internal flag when calling WithCallerOffset(3). Not sure how to handle disabling the caller feature with that approach. Setting offset to 0? -1? Just some afterthoughts :)

[MarvinJWendt]

@ccremer the offset is only required if the pterm.Logger is used inside the slog wrapper. If you use the pterm.Logger by itself, the offset is already set correctly. pterm.NewSlogHandler adds some additional function calls.

	logger := pterm.DefaultLogger.WithCaller()
	logger.Info("some info")

Works as expected.

---

We could inject more offset in pterm.NewSlogHandler, but for that, we would also need to calculate the offset, that the slog package introduces. The offset in slog could be different across Go versions, when functions inside slog are split and/or refactored. Internally, the pterm.Logger always has an offset of 4. .WithCallerOffset adds to that. That internal offset of 4 could theoretically also change between Go versions, but it's much less likely (imo). We could try to "magically" guess the offset when pterm.NewSlogHandler is used. PTerm is intended to be as easy to use as possible, without boilerplate setup code, etc. So injecting the offset in pterm.NewSlogHandler would be the nicest approach.

We could also implement that as a non-breaking change, and only add an offset in pterm.NewSlogHandler if the passed pterm.Logger does have an offset of 0. That way, existing implementations, which fixed it by adding their own offset, would not break.

[MarvinJWendt]

Changing the offset when using the slog handler, should no longer be needed in v0.12.74 (will be released as soon as the CI approved everything).

See #608, #609

[ccremer]

If you use the pterm.Logger by itself, the offset is already set correctly.

Right, I haven't thought of that ;)

Overall I'm happy with your proposal. An 80/20 approach to reduce boilerplate is probably still better than no approach.