Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Inject more CallerOffset in pterm.NewSlogHandler #608

Closed
MarvinJWendt opened this issue Jan 3, 2024 · 2 comments · Fixed by #609
Closed

Inject more CallerOffset in pterm.NewSlogHandler #608

MarvinJWendt opened this issue Jan 3, 2024 · 2 comments · Fixed by #609
Labels
proposal Proposal to add a new feature to pterm

Comments

@MarvinJWendt
Copy link
Member

As discussed in #599

The slog handler for pterm.Logger adds additional function calls, which break the default offset of pterm.Logger. We could inject more offset into the passed pterm.Logger when pterm.NewSlogHandler is called. To make this change non-breaking, we should only inject the offset, if the passed pterm.Logger has no manually configured offset.

/cc @ccremer

@MarvinJWendt MarvinJWendt added the proposal Proposal to add a new feature to pterm label Jan 3, 2024
@MarvinJWendt
Copy link
Member Author

MarvinJWendt commented Jan 3, 2024

Injecting the offset is not as easy as thought initially, since the pterm.Logger is passed as a pointer. So changing the offset in pterm.NewSlogHandler would change the offset in the logger itself, which could be used somewhere else. Passing the logger via a pointer has the advantage, that the logger configuration can be changed on the fly, e.g.:

logger := pterm.DefaultLogger.WithCaller()
logger.Info("default logger with caller")

slogger := slog.New(pterm.NewSlogHandler(logger))

slogger.Info("slog logger with caller")
logger.Info("default logger with caller 2")

slogger.Debug("This message should not print")

logger.Level = pterm.LogLevelDebug // <-- This should still be possible

slogger.Debug("This message should print")

So we need to temporarily fork the logger in the slog handler, when it is used.

@MarvinJWendt
Copy link
Member Author

Another benefit of this is, that we can change the offset if the slog package is refactored. Before that change, every repo using PTerm would have to change their manual offset.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
proposal Proposal to add a new feature to pterm
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant