Skip to content

Commit

Permalink
Merge pull request #2238 from yarpc/vitalii/release
Browse files Browse the repository at this point in the history
  • Loading branch information
biosvs authored Dec 14, 2023
2 parents 4d4c3a6 + 0f13f34 commit cd46ab0
Show file tree
Hide file tree
Showing 17 changed files with 385 additions and 60 deletions.
7 changes: 7 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,12 @@ The format is based on [Keep a Changelog](http://keepachangelog.com/en/1.0.0/)
and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0.html).

=======
## [1.71.0] - 2023-12-14
- tchannel: optional transport-level config to allow reusing a buffer for reading a tchannel response body.
- grpc: returned outbound response body is no longer writable.
- Fixed panic when error details list contains message that cannot be unmarshalled.
- Plugin v2: use v2 of internal libraries; indicate "optional" field support in the plugin response.

## [1.70.4] - 2023-08-31
- logging: fix logged error in observability middleware when fields of transport.Request is in the tagsBlocklist
- `go.mod`: update minimum requirements to go1.21 instead of go1.14 and update `golang.org/x/net v0.7.0` to v0.14.0
Expand Down Expand Up @@ -1487,6 +1493,7 @@ This release requires regeneration of ThriftRW code.
## 0.1.0 - 2016-08-31
- Initial release.
[1.71.0]: https://github.com/yarpc/yarpc-go/compare/v1.70.4...v1.71.0
[1.70.4]: https://github.com/yarpc/yarpc-go/compare/v1.70.3...v1.70.4
[1.70.3]: https://github.com/yarpc/yarpc-go/compare/v1.70.2...v1.70.3
[1.70.2]: https://github.com/yarpc/yarpc-go/compare/v1.70.1...v1.70.2
Expand Down
29 changes: 17 additions & 12 deletions encoding/protobuf/error.go
Original file line number Diff line number Diff line change
Expand Up @@ -170,25 +170,29 @@ func createStatusWithDetail(pberr *pberror, encoding transport.Encoding, codec *
}

func setApplicationErrorMeta(pberr *pberror, resw transport.ResponseWriter) {
applicationErroMetaSetter, ok := resw.(transport.ApplicationErrorMetaSetter)
applicationErrorMetaSetter, ok := resw.(transport.ApplicationErrorMetaSetter)
if !ok {
return
}

decodedDetails := GetErrorDetails(pberr)
var appErrName string
if len(decodedDetails) > 0 { // only grab the first name since this will be emitted with metrics
appErrName = messageNameWithoutPackage(proto.MessageName(
decodedDetails[0].(proto.Message)),
)
}
var (
decodedDetails = GetErrorDetails(pberr)

appErrName string
details = make([]string, 0, len(decodedDetails))
)

details := make([]string, 0, len(decodedDetails))
for _, detail := range decodedDetails {
details = append(details, protobufMessageToString(detail.(proto.Message)))
if m, ok := detail.(proto.Message); ok {
if appErrName == "" {
// only grab the first name since this will be emitted with metrics
appErrName = messageNameWithoutPackage(proto.MessageName(m))
}
details = append(details, protobufMessageToString(detail.(proto.Message)))
}
}

applicationErroMetaSetter.SetApplicationErrorMeta(&transport.ApplicationErrorMeta{
applicationErrorMetaSetter.SetApplicationErrorMeta(&transport.ApplicationErrorMeta{
Name: appErrName,
Details: fmt.Sprintf(_errDetailsFmt, strings.Join(details, " , ")),
})
Expand All @@ -198,7 +202,8 @@ func setApplicationErrorMeta(pberr *pberror, resw transport.ResponseWriter) {
// name.
//
// For example:
// uber.foo.bar.TypeName -> TypeName
//
// uber.foo.bar.TypeName -> TypeName
func messageNameWithoutPackage(messageName string) string {
if i := strings.LastIndex(messageName, "."); i >= 0 {
return messageName[i+1:]
Expand Down
20 changes: 20 additions & 0 deletions encoding/protobuf/error_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -268,3 +268,23 @@ func TestErrorHandling(t *testing.T) {
assert.Equal(t, err, fmt.Errorf("proto: Marshal called with nil"))
})
}

func TestSetApplicationErrorMeta(t *testing.T) {
respErr := NewError(
yarpcerrors.CodeAborted,
"aborted",
)

anyString, err := types.MarshalAny(&types.StringValue{Value: "baz"})
require.NoError(t, err)

pbErr := respErr.(*pberror)
pbErr.details = append(pbErr.details, &types.Any{TypeUrl: "foo", Value: []byte("bar")})
pbErr.details = append(pbErr.details, anyString)

resw := &transporttest.FakeResponseWriter{}
setApplicationErrorMeta(pbErr, resw)

assert.Equal(t, "StringValue", resw.ApplicationErrorMeta.Name)
assert.Equal(t, `[]{ StringValue{value:"baz" } }`, resw.ApplicationErrorMeta.Details)
}

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion encoding/protobuf/protoc-gen-yarpc-go-v2/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ import (
"os"

"go.uber.org/yarpc/encoding/protobuf/protoc-gen-yarpc-go-v2/internal/lib"
"go.uber.org/yarpc/internal/protoplugin"
protoplugin "go.uber.org/yarpc/internal/protoplugin-v2"
)

func main() {
Expand Down
28 changes: 17 additions & 11 deletions encoding/protobuf/v2/error.go
Original file line number Diff line number Diff line change
Expand Up @@ -167,24 +167,29 @@ func createStatusWithDetail(pberr *pberror, encoding transport.Encoding, codec *
}

func setApplicationErrorMeta(pberr *pberror, resw transport.ResponseWriter) {
applicationErroMetaSetter, ok := resw.(transport.ApplicationErrorMetaSetter)
applicationErrorMetaSetter, ok := resw.(transport.ApplicationErrorMetaSetter)
if !ok {
return
}

decodedDetails := GetErrorDetails(pberr)
var appErrName string
if len(decodedDetails) > 0 { // only grab the first name since this will be emitted with metrics
decodedMsg := decodedDetails[0].(proto.Message)
appErrName = messageNameWithoutPackage(string(proto.MessageName(decodedMsg)))
}
var (
decodedDetails = GetErrorDetails(pberr)

appErrName string
details = make([]string, 0, len(decodedDetails))
)

details := make([]string, 0, len(decodedDetails))
for _, detail := range decodedDetails {
details = append(details, protobufMessageToString(detail.(proto.Message)))
if m, ok := detail.(proto.Message); ok {
if appErrName == "" {
// only grab the first name since this will be emitted with metrics
appErrName = messageNameWithoutPackage(string(proto.MessageName(m)))
}
details = append(details, protobufMessageToString(detail.(proto.Message)))
}
}

applicationErroMetaSetter.SetApplicationErrorMeta(&transport.ApplicationErrorMeta{
applicationErrorMetaSetter.SetApplicationErrorMeta(&transport.ApplicationErrorMeta{
Name: appErrName,
Details: fmt.Sprintf(_errDetailsFmt, strings.Join(details, " , ")),
})
Expand All @@ -194,7 +199,8 @@ func setApplicationErrorMeta(pberr *pberror, resw transport.ResponseWriter) {
// name.
//
// For example:
// uber.foo.bar.TypeName -> TypeName
//
// uber.foo.bar.TypeName -> TypeName
func messageNameWithoutPackage(messageName string) string {
if i := strings.LastIndex(messageName, "."); i >= 0 {
return messageName[i+1:]
Expand Down
21 changes: 21 additions & 0 deletions encoding/protobuf/v2/error_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@ import (
"google.golang.org/grpc/codes"
"google.golang.org/grpc/status"
"google.golang.org/protobuf/proto"
anypb "google.golang.org/protobuf/types/known/anypb"
)

func TestNewOK(t *testing.T) {
Expand Down Expand Up @@ -272,3 +273,23 @@ func TestErrorHandling(t *testing.T) {
fmt.Errorf("proto: invalid nil source message").Error())
})
}

func TestSetApplicationErrorMeta(t *testing.T) {
respErr := NewError(
yarpcerrors.CodeAborted,
"aborted",
)

anyString, err := anypb.New(&wrappers.StringValue{Value: "baz"})
require.NoError(t, err)

pbErr := respErr.(*pberror)
pbErr.details = append(pbErr.details, &any.Any{TypeUrl: "foo", Value: []byte("bar")})
pbErr.details = append(pbErr.details, anyString)

resw := &transporttest.FakeResponseWriter{}
setApplicationErrorMeta(pbErr, resw)

assert.Equal(t, "StringValue", resw.ApplicationErrorMeta.Name)
assert.Equal(t, `[]{ StringValue{value:"baz"} }`, resw.ApplicationErrorMeta.Details)
}
174 changes: 174 additions & 0 deletions encoding/thrift/benchmark_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,174 @@
package thrift_test

import (
"context"
"math/rand"
"net"
"strconv"
"strings"
"sync"
"testing"
"time"

"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
"go.uber.org/yarpc"
"go.uber.org/yarpc/api/transport"
"go.uber.org/yarpc/internal/examples/thrift-keyvalue/keyvalue/kv"
"go.uber.org/yarpc/internal/examples/thrift-keyvalue/keyvalue/kv/keyvalueclient"
"go.uber.org/yarpc/internal/examples/thrift-keyvalue/keyvalue/kv/keyvalueserver"
"go.uber.org/yarpc/transport/tchannel"
"gonum.org/v1/gonum/stat/distuv"
)

const (
_kvServer = "callee"
_kvClient = "caller"
)

type generator struct {
norm distuv.Normal
min int
max int
}

func (g generator) next() int {
out := int(g.norm.Rand())
if out < g.min {
out = 0
}
if out > g.max {
out = g.max
}
return out
}

func BenchmarkThriftClientCallNormalDist(b *testing.B) {
handler := &keyValueHandler{}
serverAddr := newKeyValServer(b, handler)

clientNoReuse := newKeyValueClient(b, serverAddr, false)
clientWithReuse := newKeyValueClient(b, serverAddr, true)

// Create a normal distribution
g := generator{
norm: distuv.Normal{
Mu: 3 * 1024, // 3KB
Sigma: 10000,
},
min: 0,
max: 2 * 1024 * 1024, // 2MB
}

var samples []string
for i := 0; i < 10000; i++ {
key := "foo" + strconv.FormatInt(int64(i), 10)
len := g.next()
value := generateRandomString(len)
samples = append(samples, value)
handler.SetValue(context.Background(), &key, &value)
}

b.ResetTimer()

b.Run("with_buffer_pool", func(b *testing.B) {
b.ReportAllocs()
for i := 0; i < b.N; i++ {
offset := i % len(samples)
key := "foo" + strconv.FormatInt(int64(offset), 10)
value := samples[i%len(samples)]
callGetter(b, clientWithReuse, key, value)
}
})

b.Run("without_buffer_pool", func(b *testing.B) {
b.ReportAllocs()
for i := 0; i < b.N; i++ {
offset := i % len(samples)
key := "foo" + strconv.FormatInt(int64(offset), 10)
value := samples[i%len(samples)]
callGetter(b, clientNoReuse, key, value)
}
})
}

func generateRandomString(len int) string {
var sb strings.Builder
for i := 0; i < len; i++ {
c := 'a' + rand.Intn('z'-'a')
sb.WriteByte(byte(c))
}
return sb.String()
}

func callGetter(b *testing.B, client keyvalueclient.Interface, key string, want string) {
ctx, cancel := context.WithTimeout(context.Background(), time.Second)
defer cancel()

got, err := client.GetValue(ctx, &key)
require.NoError(b, err)
require.Equal(b, want, got)
}

type keyValueHandler struct {
items sync.Map
}

func (h *keyValueHandler) GetValue(ctx context.Context, key *string) (string, error) {
if v, ok := h.items.Load(*key); ok {
return v.(string), nil
}
return "", &kv.ResourceDoesNotExist{Key: *key}
}

func (h *keyValueHandler) SetValue(ctx context.Context, key *string, value *string) error {
h.items.Store(*key, *value)
return nil
}

func newKeyValServer(t testing.TB, handler keyvalueserver.Interface) string {
listen, err := net.Listen("tcp", "127.0.0.1:0")
require.NoError(t, err)

trans, err := tchannel.NewTransport(
tchannel.ServiceName(_kvServer),
tchannel.Listener(listen))
require.NoError(t, err)

inbound := trans.NewInbound()
addr := listen.Addr().String()

dispatcher := yarpc.NewDispatcher(yarpc.Config{
Name: _kvServer,
Inbounds: yarpc.Inbounds{inbound},
})

dispatcher.Register(keyvalueserver.New(handler))
require.NoError(t, dispatcher.Start(), "could not start server dispatcher")

t.Cleanup(func() { assert.NoError(t, dispatcher.Stop(), "could not stop dispatcher") })

return addr
}

func newKeyValueClient(t testing.TB, serverAddr string, enableBufferReuse bool) keyvalueclient.Interface {
trans, err := tchannel.NewTransport(tchannel.ServiceName(_kvClient))
require.NoError(t, err)
out := trans.NewSingleOutbound(serverAddr, tchannel.WithReuseBuffer(enableBufferReuse))

dispatcher := yarpc.NewDispatcher(yarpc.Config{
Name: _kvClient,
Outbounds: map[string]transport.Outbounds{
_kvServer: {
ServiceName: _kvServer,
Unary: out,
},
},
})

client := keyvalueclient.New(dispatcher.ClientConfig(_kvServer))
require.NoError(t, dispatcher.Start(), "could not start client dispatcher")

t.Cleanup(func() { assert.NoError(t, dispatcher.Stop(), "could not stop dispatcher") })
return client
}
7 changes: 5 additions & 2 deletions go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -26,10 +26,12 @@ require (
go.uber.org/net/metrics v1.3.0
go.uber.org/thriftrw v1.29.2
go.uber.org/tools v0.0.0-20190618225709-2cfd321de3ee
go.uber.org/yarpc/internal/examples v0.0.0-20230831212929-ccef8c01afa8
go.uber.org/zap v1.13.0
golang.org/x/lint v0.0.0-20200130185559-910be7a94367
golang.org/x/net v0.14.0
golang.org/x/tools v0.6.0
golang.org/x/tools v0.7.0
gonum.org/v1/gonum v0.14.0
google.golang.org/genproto v0.0.0-20200526211855-cb27e3aa2013
google.golang.org/grpc v1.40.1
google.golang.org/protobuf v1.26.0
Expand Down Expand Up @@ -60,8 +62,9 @@ require (
github.com/uber-common/bark v1.2.1 // indirect
github.com/uber/jaeger-lib v2.4.1+incompatible // indirect
go.uber.org/dig v1.8.0 // indirect
golang.org/x/exp v0.0.0-20230321023759-10a507213a29 // indirect
golang.org/x/exp/typeparams v0.0.0-20220218215828-6cf2b201936e // indirect
golang.org/x/mod v0.8.0 // indirect
golang.org/x/mod v0.9.0 // indirect
golang.org/x/sys v0.11.0 // indirect
golang.org/x/text v0.12.0 // indirect
)
Loading

0 comments on commit cd46ab0

Please sign in to comment.