thriftbp: Support native slog.LogValuer implementation for errors #641
Conversation
fishy
requested review from
konradreiche and
pacejackson
and removed request for
a team
go.mod
Outdated
| @@ -1,6 +1,6 @@ | |||
| module github.com/reddit/baseplate.go | |||
|
|
|||
| go 1.20 | |||
| go 1.21 | |||
There was a problem hiding this comment.
I think we should wait until 1.22 is fully released. I don't expect 1.22 until next month, right?
There was a problem hiding this comment.
I disagree that baseplate.go needs to follow go release cycle strictly. if some service is still using go 1.20, they are very unlikely to upgrade baseplate.go version anyways, and if upgrading baseplate.go version breaks them it's time for them to upgrade to go 1.21 as well.
we certainly should not drop support to a go version as soon as the next version is released, but dropping support at the end of the lifecycle to a go version is appropriate. 1.20 is also unlike 1.19 that the next version has some performance regressions.
There was a problem hiding this comment.
Well, the service foundation team does want baseplate to follow the upstream Go release cycle :). Maybe not "strictly," but definitely not more strictly than the upstream.
fishy
force-pushed
the
baseplate-error-slog
branch
from
03cab2b
to
0fa309e
Compare
|
💇 @kylelemons go 1.22 is released today :) |
|
(will rerun github actions once the 1.22 image is ready) |
Split from #641 to fix 1.22 specific problems.
fishy
force-pushed
the
baseplate-error-slog
branch
from
0fa309e
to
f43db5c
Compare
Split from #641 to fix 1.22 specific problems.
fishy
force-pushed
the
baseplate-error-slog
branch
from
f43db5c
to
b6e4268
Compare
The next thrift release, 0.20.0, will add slog.LogValuer for all thrift generated errors [1] [2]. This prepares us so that WrapBaseplateError will keep the implementation if it's available to be used with slog. [1]: apache/thrift#2884 [2]: https://issues.apache.org/jira/browse/THRIFT-5745
fishy
force-pushed
the
baseplate-error-slog
branch
from
b6e4268
to
85ae4ab
Compare
The next thrift release, 0.20.0, will add slog.LogValuer for all thrift generated errors 1 2. This prepares us so that WrapBaseplateError will keep the implementation if it's available to be used with slog.
This also drops support to go 1.20. 1.22 is already at rc2 today so I think it's time to do that.