-
Notifications
You must be signed in to change notification settings - Fork 78
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
Add SlogSink support to funcr #241
Changes from all commits
cdb930d
d95f9b6
8221825
955d2aa
44c6ac8
b5e7d9f
83dbe72
f558531
6432877
b228ba8
98ee9d9
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,142 @@ | ||
//go:build go1.21 | ||
// +build go1.21 | ||
|
||
/* | ||
Copyright 2021 The logr Authors. | ||
|
||
Licensed under the Apache License, Version 2.0 (the "License"); | ||
you may not use this file except in compliance with the License. | ||
You may obtain a copy of the License at | ||
|
||
http://www.apache.org/licenses/LICENSE-2.0 | ||
|
||
Unless required by applicable law or agreed to in writing, software | ||
distributed under the License is distributed on an "AS IS" BASIS, | ||
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. | ||
See the License for the specific language governing permissions and | ||
limitations under the License. | ||
*/ | ||
|
||
package logr | ||
|
||
import ( | ||
"log/slog" | ||
"os" | ||
"testing" | ||
|
||
"github.com/go-logr/logr" | ||
) | ||
|
||
// | ||
// slogSink wrapper of discard | ||
// | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Did you learn anything from these benchmarks? I'm just curious. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I learned that funcr with JSON is more than 2x faster than slog's JSON handler, but has more allocs. And that the slogSink (wrapper) path is (as expected) low overhead. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I did a similar comparison between slog's JSON handler and zap (kubernetes/kubernetes@9100c3c). Slog was slower, so there was no justification for switching from zap to slog as backend in Kubernetes. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. While I have your attention (sorry, couldn't resist 😁 ), could you perhaps review the PR in Kubernetes that this commit came from? The dependency update is in another pending PR, that's the only reason it's WIP. |
||
|
||
func BenchmarkSlogSinkLogInfoOneArg(b *testing.B) { | ||
var log logr.Logger = logr.FromSlogHandler(logr.ToSlogHandler(logr.Discard())) | ||
doInfoOneArg(b, log) | ||
} | ||
|
||
func BenchmarkSlogSinkLogInfoSeveralArgs(b *testing.B) { | ||
var log logr.Logger = logr.FromSlogHandler(logr.ToSlogHandler(logr.Discard())) | ||
doInfoSeveralArgs(b, log) | ||
} | ||
|
||
func BenchmarkSlogSinkLogInfoWithValues(b *testing.B) { | ||
var log logr.Logger = logr.FromSlogHandler(logr.ToSlogHandler(logr.Discard())) | ||
doInfoWithValues(b, log) | ||
} | ||
|
||
func BenchmarkSlogSinkLogV0Info(b *testing.B) { | ||
var log logr.Logger = logr.FromSlogHandler(logr.ToSlogHandler(logr.Discard())) | ||
doV0Info(b, log) | ||
} | ||
|
||
func BenchmarkSlogSinkLogV9Info(b *testing.B) { | ||
var log logr.Logger = logr.FromSlogHandler(logr.ToSlogHandler(logr.Discard())) | ||
doV9Info(b, log) | ||
} | ||
|
||
func BenchmarkSlogSinkLogError(b *testing.B) { | ||
var log logr.Logger = logr.FromSlogHandler(logr.ToSlogHandler(logr.Discard())) | ||
doError(b, log) | ||
} | ||
|
||
func BenchmarkSlogSinkWithValues(b *testing.B) { | ||
var log logr.Logger = logr.FromSlogHandler(logr.ToSlogHandler(logr.Discard())) | ||
doWithValues(b, log) | ||
} | ||
|
||
func BenchmarkSlogSinkWithName(b *testing.B) { | ||
var log logr.Logger = logr.FromSlogHandler(logr.ToSlogHandler(logr.Discard())) | ||
doWithName(b, log) | ||
} | ||
|
||
// | ||
// slogSink wrapper of slog's JSONHandler, for comparison | ||
// | ||
|
||
func makeSlogJSONLogger() logr.Logger { | ||
devnull, _ := os.Open("/dev/null") | ||
handler := slog.NewJSONHandler(devnull, nil) | ||
return logr.FromSlogHandler(handler) | ||
} | ||
|
||
func BenchmarkSlogJSONLogInfoOneArg(b *testing.B) { | ||
var log logr.Logger = makeSlogJSONLogger() | ||
doInfoOneArg(b, log) | ||
} | ||
|
||
func BenchmarkSlogJSONLogInfoSeveralArgs(b *testing.B) { | ||
var log logr.Logger = makeSlogJSONLogger() | ||
doInfoSeveralArgs(b, log) | ||
} | ||
|
||
func BenchmarkSlogJSONLogInfoWithValues(b *testing.B) { | ||
var log logr.Logger = makeSlogJSONLogger() | ||
doInfoWithValues(b, log) | ||
} | ||
|
||
func BenchmarkSlogJSONLogV0Info(b *testing.B) { | ||
var log logr.Logger = makeSlogJSONLogger() | ||
doV0Info(b, log) | ||
} | ||
|
||
func BenchmarkSlogJSONLogV9Info(b *testing.B) { | ||
var log logr.Logger = makeSlogJSONLogger() | ||
doV9Info(b, log) | ||
} | ||
|
||
func BenchmarkSlogJSONLogError(b *testing.B) { | ||
var log logr.Logger = makeSlogJSONLogger() | ||
doError(b, log) | ||
} | ||
|
||
func BenchmarkSlogJSONLogWithValues(b *testing.B) { | ||
var log logr.Logger = makeSlogJSONLogger() | ||
doWithValues(b, log) | ||
} | ||
|
||
func BenchmarkSlogJSONWithName(b *testing.B) { | ||
var log logr.Logger = makeSlogJSONLogger() | ||
doWithName(b, log) | ||
} | ||
|
||
func BenchmarkSlogJSONWithCallDepth(b *testing.B) { | ||
var log logr.Logger = makeSlogJSONLogger() | ||
doWithCallDepth(b, log) | ||
} | ||
|
||
func BenchmarkSlogJSONLogInfoStringerValue(b *testing.B) { | ||
var log logr.Logger = makeSlogJSONLogger() | ||
doStringerValue(b, log) | ||
} | ||
|
||
func BenchmarkSlogJSONLogInfoErrorValue(b *testing.B) { | ||
var log logr.Logger = makeSlogJSONLogger() | ||
doErrorValue(b, log) | ||
} | ||
|
||
func BenchmarkSlogJSONLogInfoMarshalerValue(b *testing.B) { | ||
var log logr.Logger = makeSlogJSONLogger() | ||
doMarshalerValue(b, log) | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,79 @@ | ||
//go:build go1.21 | ||
// +build go1.21 | ||
|
||
/* | ||
Copyright 2023 The logr Authors. | ||
|
||
Licensed under the Apache License, Version 2.0 (the "License"); | ||
you may not use this file except in compliance with the License. | ||
You may obtain a copy of the License at | ||
|
||
http://www.apache.org/licenses/LICENSE-2.0 | ||
|
||
Unless required by applicable law or agreed to in writing, software | ||
distributed under the License is distributed on an "AS IS" BASIS, | ||
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. | ||
See the License for the specific language governing permissions and | ||
limitations under the License. | ||
*/ | ||
|
||
package logr_test | ||
|
||
import ( | ||
"errors" | ||
"fmt" | ||
"log/slog" | ||
"os" | ||
|
||
"github.com/go-logr/logr" | ||
"github.com/go-logr/logr/funcr" | ||
) | ||
|
||
var debugWithoutTime = &slog.HandlerOptions{ | ||
ReplaceAttr: func(groups []string, a slog.Attr) slog.Attr { | ||
if a.Key == "time" { | ||
return slog.Attr{} | ||
} | ||
return a | ||
}, | ||
Level: slog.LevelDebug, | ||
} | ||
|
||
func ExampleFromSlogHandler() { | ||
logrLogger := logr.FromSlogHandler(slog.NewTextHandler(os.Stdout, debugWithoutTime)) | ||
|
||
logrLogger.Info("hello world") | ||
logrLogger.Error(errors.New("fake error"), "ignore me") | ||
logrLogger.WithValues("x", 1, "y", 2).WithValues("str", "abc").WithName("foo").WithName("bar").V(4).Info("with values, verbosity and name") | ||
|
||
// Output: | ||
// level=INFO msg="hello world" | ||
// level=ERROR msg="ignore me" err="fake error" | ||
// level=DEBUG msg="with values, verbosity and name" x=1 y=2 str=abc logger=foo/bar | ||
} | ||
|
||
func ExampleToSlogHandler() { | ||
funcrLogger := funcr.New(func(prefix, args string) { | ||
if prefix != "" { | ||
fmt.Fprintln(os.Stdout, prefix, args) | ||
} else { | ||
fmt.Fprintln(os.Stdout, args) | ||
} | ||
}, funcr.Options{ | ||
Verbosity: 10, | ||
}) | ||
|
||
slogLogger := slog.New(logr.ToSlogHandler(funcrLogger)) | ||
slogLogger.Info("hello world") | ||
slogLogger.Error("ignore me", "err", errors.New("fake error")) | ||
slogLogger.With("x", 1, "y", 2).WithGroup("group").With("str", "abc").Warn("with values and group") | ||
|
||
slogLogger = slog.New(logr.ToSlogHandler(funcrLogger.V(int(-slog.LevelDebug)))) | ||
slogLogger.Info("info message reduced to debug level") | ||
|
||
// Output: | ||
// "level"=0 "msg"="hello world" | ||
// "msg"="ignore me" "error"=null "err"="fake error" | ||
// "level"=0 "msg"="with values and group" "x"=1 "y"=2 "group"={"str"="abc"} | ||
// "level"=4 "msg"="info message reduced to debug level" | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,42 @@ | ||
//go:build go1.21 | ||
// +build go1.21 | ||
|
||
/* | ||
Copyright 2023 The logr Authors. | ||
|
||
Licensed under the Apache License, Version 2.0 (the "License"); | ||
you may not use this file except in compliance with the License. | ||
You may obtain a copy of the License at | ||
|
||
http://www.apache.org/licenses/LICENSE-2.0 | ||
|
||
Unless required by applicable law or agreed to in writing, software | ||
distributed under the License is distributed on an "AS IS" BASIS, | ||
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. | ||
See the License for the specific language governing permissions and | ||
limitations under the License. | ||
*/ | ||
|
||
// Package main is an example of using funcr. | ||
package main | ||
|
||
import ( | ||
"log/slog" | ||
|
||
"github.com/go-logr/logr" | ||
) | ||
|
||
func doSlog(log logr.Logger) { | ||
slogger := slog.New(logr.ToSlogHandler(log.WithName("slog").WithValues("mode", "slog"))) | ||
slogExample(slogger) | ||
} | ||
|
||
func slogExample(log *slog.Logger) { | ||
log.Warn("hello", "val1", 1, "val2", map[string]int{"k": 1}) | ||
log.Info("you should see this") | ||
log.Debug("you should NOT see this") | ||
log.Error("uh oh", "trouble", true, "reasons", []float64{0.1, 0.11, 3.14}) | ||
log.With("attr1", 1, "attr2", 2).Info("with attrs") | ||
log.WithGroup("groupname").Info("with group", "slog2", false) | ||
log.WithGroup("group1").With("attr1", 1).WithGroup("group2").With("attr2", 2).Info("msg", "arg", "val") | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I agree that we should cover the new code with linting. I don't think it's worth to lint also with older Go because the non-slog code isn't that complex and going away at some point.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
i cant tell if you are agreeing with me or want a change?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I agree with you.