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.

Sign up for GitHub

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

Jump to bottom

Slog writer #46

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

Slog writer #46

wants to merge 2 commits into from

Conversation

SimonRichardson
Copy link
Member

@SimonRichardson SimonRichardson commented Aug 24, 2023 •
edited
Loading

To take advantage of the new logging package from the go 1.21 release, the loggo package starts incorporate these new changes. There are features that we could start to add to loggo to then pass directly to slog. The migration over to slog could then be done in stages.

For now, it might be useful to at least play around with slog and see if there are any advantages to it.

Running the new example, we can take advantage of slog for the backend with structured logging.

go run ./example "<root>=TRACE" slog

Current logging levels:
<root>=TRACE
Using log/slog writer:

time=2023-09-26T11:27:31.541+01:00 level=INFO msg="Start of test." module="" filename=/home/simon/go/src/github.com/juju/loggo/example/main.go line=47 foo=bar
time=2023-09-26T11:27:31.542+01:00 level=ERROR+1 msg="first critical" module=first filename=/home/simon/go/src/github.com/juju/loggo/example/first.go line=11 baz=boo
time=2023-09-26T11:27:31.542+01:00 level=ERROR msg="first error" module=first filename=/home/simon/go/src/github.com/juju/loggo/example/first.go line=15
time=2023-09-26T11:27:31.542+01:00 level=WARN msg="first warning" module=first filename=/home/simon/go/src/github.com/juju/loggo/example/first.go line=19
time=2023-09-26T11:27:31.542+01:00 level=INFO msg="first info" module=first filename=/home/simon/go/src/github.com/juju/loggo/example/first.go line=23
time=2023-09-26T11:27:31.542+01:00 level=DEBUG msg="first debug" module=first filename=/home/simon/go/src/github.com/juju/loggo/example/first.go line=27
time=2023-09-26T11:27:31.542+01:00 level=DEBUG-1 msg="first trace" module=first filename=/home/simon/go/src/github.com/juju/loggo/example/first.go line=31
time=2023-09-26T11:27:31.542+01:00 level=ERROR+1 msg="second critical" module=second filename=/home/simon/go/src/github.com/juju/loggo/example/second.go line=10
time=2023-09-26T11:27:31.542+01:00 level=ERROR msg="second error" module=second filename=/home/simon/go/src/github.com/juju/loggo/example/second.go line=14
time=2023-09-26T11:27:31.542+01:00 level=WARN msg="second warning" module=second filename=/home/simon/go/src/github.com/juju/loggo/example/second.go line=18
time=2023-09-26T11:27:31.542+01:00 level=INFO msg="second info" module=second filename=/home/simon/go/src/github.com/juju/loggo/example/second.go line=22
time=2023-09-26T11:27:31.542+01:00 level=DEBUG msg="second debug" module=second filename=/home/simon/go/src/github.com/juju/loggo/example/second.go line=26
time=2023-09-26T11:27:31.542+01:00 level=DEBUG-1 msg="second trace" module=second filename=/home/simon/go/src/github.com/juju/loggo/example/second.go line=29

This still works with namespacing as well.
Notice that first is at trace and everything else is at warning.

go run ./example "<root>=WARNING;first=TRACE" slog

Current logging levels:
<root>=WARNING;first=TRACE
Using log/slog writer:

time=2023-09-26T14:15:20.783+01:00 level=ERROR+1 msg="first critical" module=first filename=/home/simon/go/src/github.com/juju/loggo/example/first.go line=11 baz=boo
time=2023-09-26T14:15:20.783+01:00 level=ERROR msg="first error" module=first filename=/home/simon/go/src/github.com/juju/loggo/example/first.go line=15
time=2023-09-26T14:15:20.783+01:00 level=WARN msg="first warning" module=first filename=/home/simon/go/src/github.com/juju/loggo/example/first.go line=19
time=2023-09-26T14:15:20.783+01:00 level=INFO msg="first info" module=first filename=/home/simon/go/src/github.com/juju/loggo/example/first.go line=23
time=2023-09-26T14:15:20.783+01:00 level=DEBUG msg="first debug" module=first filename=/home/simon/go/src/github.com/juju/loggo/example/first.go line=27
time=2023-09-26T14:15:20.783+01:00 level=DEBUG-1 msg="first trace" module=first filename=/home/simon/go/src/github.com/juju/loggo/example/first.go line=31
time=2023-09-26T14:15:20.783+01:00 level=ERROR+1 msg="second critical" module=second filename=/home/simon/go/src/github.com/juju/loggo/example/second.go line=10
time=2023-09-26T14:15:20.783+01:00 level=ERROR msg="second error" module=second filename=/home/simon/go/src/github.com/juju/loggo/example/second.go line=14
time=2023-09-26T14:15:20.783+01:00 level=WARN msg="second warning" module=second filename=/home/simon/go/src/github.com/juju/loggo/example/second.go line=18

@SimonRichardson
Slog writer
54a1038 
To take advantage of the new logging package from the go 1.21 release,
the loggo package starts incorporate these new changes. There are
features that we could start to add to loggo to then pass directly to
slog. The migration over to slog could then be done in stages.

For now, it might be useful to at least play around with slog and see
if there are any advantages to it.
@SimonRichardson SimonRichardson self-assigned this Aug 24, 2023
tlm
tlm approved these changes Sep 13, 2023
View reviewed changes
Copy link
Member

@tlm tlm left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM,

How do you think this could work with our filtering logic for logs from the client? Do you see a path forward that would allow us to use slog interface directly over loggo ?

Tests would be good as well.

slog/writer.go
case loggo.CRITICAL:
return slog.LevelError + 1
default:
panic("unknown level")
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is the only part I am a bit iffy about. Don't feel comfortable that there exists a case in a library where we can blow up. Can we just default off to a sane log level?

I know it's a low chance...

@SimonRichardson
Implement attributes to the loggo package
c49e5da 
In order to correctly interface with the log/slog package and to
take advantage of structured logging, we now introduce attributes.
Currently Info and Infof are different, in the fact that Info is
the structured variation. It expects every argument to be a attribute
after the initial message string.

This then allows us to have a slog backend without migrating away
from loggo directly. We have all the benefits of loggo plus we can
take advantage of the new logging infra.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@tlm tlm tlm approved these changes

Assignees

@SimonRichardson SimonRichardson

Labels
do not merge enhancement
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@SimonRichardson @tlm