Skip to content

Commit

Permalink
Update grpc to v1.66.0 and switch to CodecV2 (#5900)
Browse files Browse the repository at this point in the history
gRPC v1.66.0 introduced CodecV2 that supports buffer pooling. The old V1
codecs no longer work (just the upgrade itself was causing panic, see
grpc/grpc-go@cfd14ba#r145974307)

This PR contains the following updates:

* Upgrade `pkg/gogocodec/codec.go` to implement CodecV2 API.
* Translate `mem.BufferSlice` into `[]byte` as needed (introduces a very
small performance hit and increase in allocations)

| Package | Change | Age | Adoption | Passing | Confidence |
|---|---|---|---|---|---|
| [google.golang.org/grpc](https://togithub.com/grpc/grpc-go) |
`v1.65.0` -> `v1.66.0` |
[![age](https://developer.mend.io/api/mc/badges/age/go/google.golang.org%2fgrpc/v1.66.0?slim=true)](https://docs.renovatebot.com/merge-confidence/)
|
[![adoption](https://developer.mend.io/api/mc/badges/adoption/go/google.golang.org%2fgrpc/v1.66.0?slim=true)](https://docs.renovatebot.com/merge-confidence/)
|
[![passing](https://developer.mend.io/api/mc/badges/compatibility/go/google.golang.org%2fgrpc/v1.65.0/v1.66.0?slim=true)](https://docs.renovatebot.com/merge-confidence/)
|
[![confidence](https://developer.mend.io/api/mc/badges/confidence/go/google.golang.org%2fgrpc/v1.65.0/v1.66.0?slim=true)](https://docs.renovatebot.com/merge-confidence/)
|

## Performance differences
Before
```
$ go test -benchmem -run=^$ -bench ^BenchmarkCodec github.com/jaegertracing/jaeger/pkg/gogocodec
goos: darwin
goarch: arm64
pkg: github.com/jaegertracing/jaeger/pkg/gogocodec
cpu: Apple M1 Pro
BenchmarkCodecUnmarshal25Spans-10    	   30015	     39020 ns/op	  108616 B/op	    1082 allocs/op
```
After
```
goos: darwin
goarch: arm64
pkg: github.com/jaegertracing/jaeger/pkg/gogocodec
cpu: Apple M1 Pro
BenchmarkCodecUnmarshal25Spans-10    	   29352	     39752 ns/op	  119496 B/op	    1083 allocs/op
```
---

### Release Notes

<details>
<summary>grpc/grpc-go (google.golang.org/grpc)</summary>

### [`v1.66.0`](https://togithub.com/grpc/grpc-go/releases/tag/v1.66.0):
Release 1.66.0

[Compare
Source](https://togithub.com/grpc/grpc-go/compare/v1.65.0...v1.66.0)

### New Features

- metadata: stabilize `ValueFromIncomingContext`
([#&#8203;7368](https://togithub.com/grpc/grpc-go/issues/7368))
- Special Thanks:
[@&#8203;KarthikReddyPuli](https://togithub.com/KarthikReddyPuli)
- client: stabilize the `WaitForStateChange` and `GetState` methods,
which were previously experimental.
([#&#8203;7425](https://togithub.com/grpc/grpc-go/issues/7425))
- xds: Implement ADS flow control mechanism
([#&#8203;7458](https://togithub.com/grpc/grpc-go/issues/7458))
- See
[https://github.com/grpc/grpc/issues/34099](https://togithub.com/grpc/grpc/issues/34099)
for context.
- balancer/rls: Add metrics for data cache and picker internals
([#&#8203;7484](https://togithub.com/grpc/grpc-go/issues/7484),
[#&#8203;7495](https://togithub.com/grpc/grpc-go/issues/7495))
- xds: LRS load reports now include the `total_issued_requests` field.
([#&#8203;7544](https://togithub.com/grpc/grpc-go/issues/7544))

### Bug Fixes

- grpc: Clients now return status code INTERNAL instead of UNIMPLEMENTED
when the server uses an unsupported compressor. This is consistent with
the [gRPC compression
spec](https://togithub.com/grpc/grpc/blob/master/doc/compression.md#compression-method-asymmetry-between-peers).
([#&#8203;7461](https://togithub.com/grpc/grpc-go/issues/7461))
- Special Thanks:
[@&#8203;Gayathri625](https://togithub.com/Gayathri625)
- transport: Fix a bug which could result in writes busy looping when
the underlying `conn.Write` returns errors
([#&#8203;7394](https://togithub.com/grpc/grpc-go/issues/7394))
    -   Special Thanks: [@&#8203;veshij](https://togithub.com/veshij)
- client: fix race that could lead to orphaned connections and
associated resources.
([#&#8203;7390](https://togithub.com/grpc/grpc-go/issues/7390))
- xds: use locality from the connected address for load reporting with
pick_first
([#&#8203;7378](https://togithub.com/grpc/grpc-go/issues/7378))
- without this fix, if a priority contains multiple localities with
pick_first, load was reported for the wrong locality
- client: prevent hanging during ClientConn.Close() when the network is
unreachable
([#&#8203;7540](https://togithub.com/grpc/grpc-go/issues/7540))

### Performance Improvements

- transport: double buffering is avoided when using an http connect
proxy and the target server waits for client to send the first message.
([#&#8203;7424](https://togithub.com/grpc/grpc-go/issues/7424))
- codec: Implement a new `Codec` which uses buffer recycling for encoded
message ([#&#8203;7356](https://togithub.com/grpc/grpc-go/issues/7356))
- introduce a `mem` package to facilitate buffer reuse
([#&#8203;7432](https://togithub.com/grpc/grpc-go/issues/7432))
- Special Thanks:
[@&#8203;PapaCharlie](https://togithub.com/PapaCharlie)

</details>

---

### Configuration

📅 **Schedule**: Branch creation - At any time (no schedule defined),
Automerge - At any time (no schedule defined).

🚦 **Automerge**: Disabled by config. Please merge this manually once you
are satisfied.

♻ **Rebasing**: Whenever PR becomes conflicted, or you tick the
rebase/retry checkbox.

🔕 **Ignore**: Close this PR and you won't be reminded about this update
again.

---

- [ ] <!-- rebase-check -->If you want to rebase/retry this PR, check
this box

---

This PR was generated by [Mend Renovate](https://mend.io/renovate/).
View the [repository job
log](https://developer.mend.io/github/jaegertracing/jaeger).

<!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiIzOC41Ni4wIiwidXBkYXRlZEluVmVyIjoiMzguNTYuMCIsInRhcmdldEJyYW5jaCI6Im1haW4iLCJsYWJlbHMiOlsiY2hhbmdlbG9nOmRlcGVuZGVuY2llcyJdfQ==-->

---------

Signed-off-by: Mend Renovate <[email protected]>
Signed-off-by: Yuri Shkuro <[email protected]>
Co-authored-by: Yuri Shkuro <[email protected]>
Co-authored-by: Yuri Shkuro <[email protected]>
  • Loading branch information
3 people authored Aug 29, 2024
1 parent d520c41 commit c0449f4
Show file tree
Hide file tree
Showing 5 changed files with 51 additions and 17 deletions.
4 changes: 2 additions & 2 deletions cmd/es-index-cleaner/README.md
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
# es-index-cleaner
# jaeger-es-index-cleaner

It is common to only keep observability data for a limited time.
However, Elasticsearch does no support expiring of old data via TTL.
To help with this task, `es-index-cleaner` can be used to purge
To help with this task, `jaeger-es-index-cleaner` can be used to purge
old Jaeger indices. For example, to delete indixes older than 14 days:

```
Expand Down
2 changes: 1 addition & 1 deletion go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -90,7 +90,7 @@ require (
go.uber.org/zap v1.27.0
golang.org/x/net v0.28.0
golang.org/x/sys v0.24.0
google.golang.org/grpc v1.65.0
google.golang.org/grpc v1.66.0
google.golang.org/protobuf v1.34.2
gopkg.in/yaml.v3 v3.0.1
)
Expand Down
10 changes: 6 additions & 4 deletions go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -109,8 +109,8 @@ github.com/emicklei/go-restful/v3 v3.11.0/go.mod h1:6n3XBCmQQb25CM2LCACGz8ukIrRr
github.com/envoyproxy/go-control-plane v0.9.0/go.mod h1:YTl/9mNaCwkRvm6d1a2C3ymFceY/DCBVvsKhRF0iEA4=
github.com/envoyproxy/go-control-plane v0.9.1-0.20191026205805-5f8ba28d4473/go.mod h1:YTl/9mNaCwkRvm6d1a2C3ymFceY/DCBVvsKhRF0iEA4=
github.com/envoyproxy/go-control-plane v0.9.4/go.mod h1:6rpuAdCZL397s3pYoYcLgu1mIlRU8Am5FuJP05cCM98=
github.com/envoyproxy/go-control-plane v0.12.0 h1:4X+VP1GHd1Mhj6IB5mMeGbLCleqxjletLK6K0rbxyZI=
github.com/envoyproxy/go-control-plane v0.12.0/go.mod h1:ZBTaoJ23lqITozF0M6G4/IragXCQKCnYbmlmtHvwRG0=
github.com/envoyproxy/go-control-plane v0.12.1-0.20240621013728-1eb8caab5155 h1:IgJPqnrlY2Mr4pYB6oaMKvFvwJ9H+X6CCY5x1vCTcpc=
github.com/envoyproxy/go-control-plane v0.12.1-0.20240621013728-1eb8caab5155/go.mod h1:5Wkq+JduFtdAXihLmeTJf+tRYIT4KBc2vPXDhwVo1pA=
github.com/envoyproxy/protoc-gen-validate v0.1.0/go.mod h1:iSmxcyjqTsJpI2R4NaDN7+kN2VEUnK/pcBlmesArF7c=
github.com/envoyproxy/protoc-gen-validate v1.0.4 h1:gVPz/FMfvh57HdSJQyvBtF00j8JU4zdyUgIUNhlgg0A=
github.com/envoyproxy/protoc-gen-validate v1.0.4/go.mod h1:qys6tmnRsYrQqIhm2bvKZH4Blx/1gTIZ2UKVY1M+Yew=
Expand Down Expand Up @@ -438,6 +438,8 @@ github.com/pkg/browser v0.0.0-20240102092130-5ac0b6a4141c/go.mod h1:7rwL4CYBLnjL
github.com/pkg/diff v0.0.0-20210226163009-20ebb0f2a09e/go.mod h1:pJLUxLENpZxwdsKMEsNbx1VGcRFpLqf3715MtcvvzbA=
github.com/pkg/errors v0.9.1 h1:FEBLx1zS214owpjy7qsBeixbURkuhQAwrK5UwLGTwt4=
github.com/pkg/errors v0.9.1/go.mod h1:bwawxfHBFNV+L2hUp1rHADufV3IMtnDRdf1r5NINEl0=
github.com/planetscale/vtprotobuf v0.6.1-0.20240319094008-0393e58bdf10 h1:GFCKgmp0tecUJ0sJuv4pzYCqS9+RGSn52M3FUwPs+uo=
github.com/planetscale/vtprotobuf v0.6.1-0.20240319094008-0393e58bdf10/go.mod h1:t/avpk3KcrXxUnYOhZhMXJlSEyie6gQbtLq5NM3loB8=
github.com/pmezard/go-difflib v1.0.0/go.mod h1:iKH77koFhYxTK1pcRnkKkqfTogsbg7gZNVY4sRDYZ/4=
github.com/pmezard/go-difflib v1.0.1-0.20181226105442-5d4384ee4fb2 h1:Jamvg5psRIccs7FGNTlIRMkT8wgtp5eCXdBlqhYGL6U=
github.com/pmezard/go-difflib v1.0.1-0.20181226105442-5d4384ee4fb2/go.mod h1:iKH77koFhYxTK1pcRnkKkqfTogsbg7gZNVY4sRDYZ/4=
Expand Down Expand Up @@ -838,8 +840,8 @@ google.golang.org/grpc v1.23.0/go.mod h1:Y5yQAOtifL1yxbo5wqy6BxZv8vAUGQwXBOALyac
google.golang.org/grpc v1.25.1/go.mod h1:c3i+UQWmh7LiEpx4sFZnkU36qjEYZ0imhYfXVyQciAY=
google.golang.org/grpc v1.27.0/go.mod h1:qbnxyOmOxrQa7FizSgH+ReBfzJrCY1pSN7KXBS8abTk=
google.golang.org/grpc v1.33.2/go.mod h1:JMHMWHQWaTccqQQlmk3MJZS+GWXOdAesneDmEnv2fbc=
google.golang.org/grpc v1.65.0 h1:bs/cUb4lp1G5iImFFd3u5ixQzweKizoZJAwBNLR42lc=
google.golang.org/grpc v1.65.0/go.mod h1:WgYC2ypjlB0EiQi6wdKixMqukr6lBc0Vo+oOgjrM5ZQ=
google.golang.org/grpc v1.66.0 h1:DibZuoBznOxbDQxRINckZcUvnCEvrW9pcWIE2yF9r1c=
google.golang.org/grpc v1.66.0/go.mod h1:s3/l6xSSCURdVfAnL+TqCNMyTDAGN6+lZeVxnZR128Y=
google.golang.org/protobuf v0.0.0-20200109180630-ec00e32a8dfd/go.mod h1:DFci5gLYBciE7Vtevhsrf46CRTquxDuWsQurQQe4oz8=
google.golang.org/protobuf v0.0.0-20200221191635-4d8936d0db64/go.mod h1:kwYJMbMJ01Woi6D6+Kah6886xMZcty6N08ah7+eCXa0=
google.golang.org/protobuf v0.0.0-20200228230310-ab0ca4ff8a60/go.mod h1:cfTl7dwQJ+fmap5saPgwCLgHXTUD7jkjRqWcaiX5VyM=
Expand Down
18 changes: 10 additions & 8 deletions pkg/gogocodec/codec.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,14 +11,15 @@ import (
gogoproto "github.com/gogo/protobuf/proto"
"google.golang.org/grpc/encoding"
"google.golang.org/grpc/encoding/proto"
"google.golang.org/grpc/mem"
)

const (
jaegerProtoGenPkgPath = "github.com/jaegertracing/jaeger/proto-gen"
jaegerModelPkgPath = "github.com/jaegertracing/jaeger/model"
)

var defaultCodec encoding.Codec
var defaultCodec encoding.CodecV2

// CustomType is an interface that Gogo expects custom types to implement.
// https://github.com/gogo/protobuf/blob/master/custom_types.md
Expand All @@ -34,16 +35,16 @@ type CustomType interface {
}

func init() {
defaultCodec = encoding.GetCodec(proto.Name)
defaultCodec = encoding.GetCodecV2(proto.Name)
defaultCodec.Name() // ensure it's not nil
encoding.RegisterCodec(newCodec())
encoding.RegisterCodecV2(newCodec())
}

// gogoCodec forces the use of gogo proto marshalling/unmarshalling for
// Jaeger proto types (package jaeger/gen-proto).
type gogoCodec struct{}

var _ encoding.Codec = (*gogoCodec)(nil)
var _ encoding.CodecV2 = (*gogoCodec)(nil)

func newCodec() *gogoCodec {
return &gogoCodec{}
Expand All @@ -55,23 +56,24 @@ func (*gogoCodec) Name() string {
}

// Marshal implements encoding.Codec
func (*gogoCodec) Marshal(v any) ([]byte, error) {
func (*gogoCodec) Marshal(v any) (mem.BufferSlice, error) {
t := reflect.TypeOf(v)
elem := t.Elem()
// use gogo proto only for Jaeger types
if useGogo(elem) {
return gogoproto.Marshal(v.(gogoproto.Message))
bytes, err := gogoproto.Marshal(v.(gogoproto.Message))
return mem.BufferSlice{mem.SliceBuffer(bytes)}, err
}
return defaultCodec.Marshal(v)
}

// Unmarshal implements encoding.Codec
func (*gogoCodec) Unmarshal(data []byte, v any) error {
func (*gogoCodec) Unmarshal(data mem.BufferSlice, v any) error {
t := reflect.TypeOf(v)
elem := t.Elem() // only for collections
// use gogo proto only for Jaeger types
if useGogo(elem) {
return gogoproto.Unmarshal(data, v.(gogoproto.Message))
return gogoproto.Unmarshal(data.Materialize(), v.(gogoproto.Message))
}
return defaultCodec.Unmarshal(data, v)
}
Expand Down
34 changes: 32 additions & 2 deletions pkg/gogocodec/codec_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,11 +4,14 @@
package gogocodec

import (
"os"
"reflect"
"testing"

"github.com/gogo/protobuf/jsonpb"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
"google.golang.org/grpc/mem"
"google.golang.org/protobuf/proto"
"google.golang.org/protobuf/types/known/emptypb"
"google.golang.org/protobuf/types/known/timestamppb"
Expand Down Expand Up @@ -54,14 +57,14 @@ func TestWireCompatibility(t *testing.T) {
require.NoError(t, err)

var goprotoMessage emptypb.Empty
err = proto.Unmarshal(data, &goprotoMessage)
err = proto.Unmarshal(data.Materialize(), &goprotoMessage)
require.NoError(t, err)

data2, err := proto.Marshal(&goprotoMessage)
require.NoError(t, err)

s2 := &model.Span{}
err = c.Unmarshal(data2, s2)
err = c.Unmarshal(mem.BufferSlice{mem.SliceBuffer(data2)}, s2)
require.NoError(t, err)
assert.Equal(t, s1, s2)
}
Expand All @@ -73,6 +76,33 @@ func TestUseGogo(t *testing.T) {
assert.True(t, useGogo(reflect.TypeOf(span)))
}

func BenchmarkCodecUnmarshal25Spans(b *testing.B) {
const fileName = "../../model/converter/thrift/jaeger/fixtures/domain_01.json"
jsonFile, err := os.Open(fileName)
require.NoError(b, err, "Failed to open json fixture file %s", fileName)
var trace model.Trace
require.NoError(b, jsonpb.Unmarshal(jsonFile, &trace), fileName)
require.NotEmpty(b, trace.Spans)
spans := make([]*model.Span, 25)
for i := 0; i < len(spans); i++ {
spans[i] = trace.Spans[0]
}
trace.Spans = spans
c := newCodec()
bytes, err := c.Marshal(&trace)
require.NoError(b, err)

b.ResetTimer()

for i := 0; i < b.N; i++ {
var trace model.Trace
err := c.Unmarshal(bytes, &trace)
if err != nil {
b.Fatal(err)
}
}
}

func TestMain(m *testing.M) {
testutils.VerifyGoLeaks(m)
}

0 comments on commit c0449f4

Please sign in to comment.