Skip to content

deps: move from github.com/golang/protobuf to google.golang.org/protobuf/proto #7059

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

Merged
merged 10 commits into from
Apr 5, 2024
Merged

deps: move from github.com/golang/protobuf to google.golang.org/protobuf/proto #7059

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
2f40596
move channelz to google.golang.org/protobuf
Clement-Jean Mar 21, 2024
4dc482f
check timestamp validity
Clement-Jean Mar 21, 2024
21fd935
replace proto with prototext and protoimpl
Clement-Jean Mar 26, 2024
5d2ed71
Merge branch 'master' into remove_old_proto_pkg
Clement-Jean Mar 27, 2024
18d5f31
move to well-known types and solve conflict
Clement-Jean Mar 27, 2024
5d342be
rm protoimpl
Clement-Jean Mar 27, 2024
54a4823
remove ptypes
Clement-Jean Apr 2, 2024
a8f1787
update check for github.com/golang/protobuf
Clement-Jean Apr 2, 2024
39fb9b5
remove flaky test by using proto file
Clement-Jean Apr 3, 2024
190126d
rm proto file and create ProtoReflect function
Clement-Jean Apr 4, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 6 additions & 7 deletions channelz/service/func_linux.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,15 +21,14 @@ package service
import (
"time"

"github.com/golang/protobuf/ptypes"
durpb "github.com/golang/protobuf/ptypes/duration"
channelzpb "google.golang.org/grpc/channelz/grpc_channelz_v1"
"google.golang.org/grpc/internal/channelz"
"google.golang.org/protobuf/types/known/anypb"
"google.golang.org/protobuf/types/known/durationpb"
)

func convertToPtypesDuration(sec int64, usec int64) *durpb.Duration {
return ptypes.DurationProto(time.Duration(sec*1e9 + usec*1e3))
func convertToPbDuration(sec int64, usec int64) *durationpb.Duration {
return durationpb.New(time.Duration(sec*1e9 + usec*1e3))
}

func sockoptToProto(skopts *channelz.SocketOptionData) []*channelzpb.SocketOption {
Expand All @@ -40,7 +39,7 @@ func sockoptToProto(skopts *channelz.SocketOptionData) []*channelzpb.SocketOptio
if skopts.Linger != nil {
additional, err := anypb.New(&channelzpb.SocketOptionLinger{
Active: skopts.Linger.Onoff != 0,
Duration: convertToPtypesDuration(int64(skopts.Linger.Linger), 0),
Duration: convertToPbDuration(int64(skopts.Linger.Linger), 0),
})
if err == nil {
opts = append(opts, &channelzpb.SocketOption{
Expand All @@ -53,7 +52,7 @@ func sockoptToProto(skopts *channelz.SocketOptionData) []*channelzpb.SocketOptio
}
if skopts.RecvTimeout != nil {
additional, err := anypb.New(&channelzpb.SocketOptionTimeout{
Duration: convertToPtypesDuration(int64(skopts.RecvTimeout.Sec), int64(skopts.RecvTimeout.Usec)),
Duration: convertToPbDuration(int64(skopts.RecvTimeout.Sec), int64(skopts.RecvTimeout.Usec)),
})
if err == nil {
opts = append(opts, &channelzpb.SocketOption{
Expand All @@ -66,7 +65,7 @@ func sockoptToProto(skopts *channelz.SocketOptionData) []*channelzpb.SocketOptio
}
if skopts.SendTimeout != nil {
additional, err := anypb.New(&channelzpb.SocketOptionTimeout{
Duration: convertToPtypesDuration(int64(skopts.SendTimeout.Sec), int64(skopts.SendTimeout.Usec)),
Duration: convertToPbDuration(int64(skopts.SendTimeout.Sec), int64(skopts.SendTimeout.Usec)),
})
if err == nil {
opts = append(opts, &channelzpb.SocketOption{
Expand Down
29 changes: 14 additions & 15 deletions channelz/service/service.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,8 +24,6 @@ import (
"net"
"time"

"github.com/golang/protobuf/ptypes"
wrpb "github.com/golang/protobuf/ptypes/wrappers"
channelzgrpc "google.golang.org/grpc/channelz/grpc_channelz_v1"
channelzpb "google.golang.org/grpc/channelz/grpc_channelz_v1"

Expand All @@ -36,8 +34,9 @@ import (
"google.golang.org/grpc/grpclog"
"google.golang.org/grpc/internal/channelz"
"google.golang.org/grpc/status"
"google.golang.org/protobuf/protoadapt"
"google.golang.org/protobuf/types/known/anypb"
"google.golang.org/protobuf/types/known/timestamppb"
"google.golang.org/protobuf/types/known/wrapperspb"
)

func init() {
Expand Down Expand Up @@ -89,7 +88,7 @@ func channelTraceToProto(ct *channelz.ChannelTrace) *channelzpb.ChannelTrace {
return pbt
}
pbt.NumEventsLogged = ct.EventNum
if ts, err := ptypes.TimestampProto(ct.CreationTime); err == nil {
if ts := timestamppb.New(ct.CreationTime); ts.IsValid() {
pbt.CreationTimestamp = ts
}
events := make([]*channelzpb.ChannelTraceEvent, 0, len(ct.Events))
Expand All @@ -98,7 +97,7 @@ func channelTraceToProto(ct *channelz.ChannelTrace) *channelzpb.ChannelTrace {
Description: e.Desc,
Severity: channelzpb.ChannelTraceEvent_Severity(e.Severity),
}
if ts, err := ptypes.TimestampProto(e.Timestamp); err == nil {
if ts := timestamppb.New(e.Timestamp); ts.IsValid() {
cte.Timestamp = ts
}
if e.RefID != 0 {
Expand Down Expand Up @@ -133,7 +132,7 @@ func channelMetricToProto(cm *channelz.Channel) *channelzpb.Channel {
CallsSucceeded: cm.ChannelMetrics.CallsSucceeded.Load(),
CallsFailed: cm.ChannelMetrics.CallsFailed.Load(),
}
if ts, err := ptypes.TimestampProto(time.Unix(0, cm.ChannelMetrics.LastCallStartedTimestamp.Load())); err == nil {
if ts := timestamppb.New(time.Unix(0, cm.ChannelMetrics.LastCallStartedTimestamp.Load())); ts.IsValid() {
c.Data.LastCallStartedTimestamp = ts
}
ncs := cm.NestedChans()
Expand Down Expand Up @@ -165,7 +164,7 @@ func subChannelMetricToProto(cm *channelz.SubChannel) *channelzpb.Subchannel {
CallsSucceeded: cm.ChannelMetrics.CallsSucceeded.Load(),
CallsFailed: cm.ChannelMetrics.CallsFailed.Load(),
}
if ts, err := ptypes.TimestampProto(time.Unix(0, cm.ChannelMetrics.LastCallStartedTimestamp.Load())); err == nil {
if ts := timestamppb.New(time.Unix(0, cm.ChannelMetrics.LastCallStartedTimestamp.Load())); ts.IsValid() {
sc.Data.LastCallStartedTimestamp = ts
}

Expand All @@ -191,7 +190,7 @@ func securityToProto(se credentials.ChannelzSecurityValue) *channelzpb.Security
otherSecurity := &channelzpb.Security_OtherSecurity{
Name: v.Name,
}
if anyval, err := anypb.New(protoadapt.MessageV2Of(v.Value)); err == nil {
if anyval, err := anypb.New(v.Value); err == nil {
otherSecurity.Value = anyval
}
return &channelzpb.Security{Model: &channelzpb.Security_Other{Other: otherSecurity}}
Expand Down Expand Up @@ -234,22 +233,22 @@ func socketMetricToProto(skt *channelz.Socket) *channelzpb.Socket {
MessagesReceived: skt.SocketMetrics.MessagesReceived.Load(),
KeepAlivesSent: skt.SocketMetrics.KeepAlivesSent.Load(),
}
if ts, err := ptypes.TimestampProto(time.Unix(0, skt.SocketMetrics.LastLocalStreamCreatedTimestamp.Load())); err == nil {
if ts := timestamppb.New(time.Unix(0, skt.SocketMetrics.LastLocalStreamCreatedTimestamp.Load())); ts.IsValid() {
s.Data.LastLocalStreamCreatedTimestamp = ts
}
if ts, err := ptypes.TimestampProto(time.Unix(0, skt.SocketMetrics.LastRemoteStreamCreatedTimestamp.Load())); err == nil {
if ts := timestamppb.New(time.Unix(0, skt.SocketMetrics.LastRemoteStreamCreatedTimestamp.Load())); ts.IsValid() {
s.Data.LastRemoteStreamCreatedTimestamp = ts
}
if ts, err := ptypes.TimestampProto(time.Unix(0, skt.SocketMetrics.LastMessageSentTimestamp.Load())); err == nil {
if ts := timestamppb.New(time.Unix(0, skt.SocketMetrics.LastMessageSentTimestamp.Load())); ts.IsValid() {
s.Data.LastMessageSentTimestamp = ts
}
if ts, err := ptypes.TimestampProto(time.Unix(0, skt.SocketMetrics.LastMessageReceivedTimestamp.Load())); err == nil {
if ts := timestamppb.New(time.Unix(0, skt.SocketMetrics.LastMessageReceivedTimestamp.Load())); ts.IsValid() {
s.Data.LastMessageReceivedTimestamp = ts
}
if skt.EphemeralMetrics != nil {
e := skt.EphemeralMetrics()
s.Data.LocalFlowControlWindow = &wrpb.Int64Value{Value: e.LocalFlowControlWindow}
s.Data.RemoteFlowControlWindow = &wrpb.Int64Value{Value: e.RemoteFlowControlWindow}
s.Data.LocalFlowControlWindow = &wrapperspb.Int64Value{Value: e.LocalFlowControlWindow}
s.Data.RemoteFlowControlWindow = &wrapperspb.Int64Value{Value: e.RemoteFlowControlWindow}
}

s.Data.Option = sockoptToProto(skt.SocketOptions)
Expand Down Expand Up @@ -280,7 +279,7 @@ func serverMetricToProto(sm *channelz.Server) *channelzpb.Server {
CallsFailed: sm.ServerMetrics.CallsFailed.Load(),
}

if ts, err := ptypes.TimestampProto(time.Unix(0, sm.ServerMetrics.LastCallStartedTimestamp.Load())); err == nil {
if ts := timestamppb.New(time.Unix(0, sm.ServerMetrics.LastCallStartedTimestamp.Load())); ts.IsValid() {
s.Data.LastCallStartedTimestamp = ts
}
lss := sm.ListenSockets()
Expand Down
38 changes: 25 additions & 13 deletions channelz/service/service_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,14 +27,18 @@ import (
"testing"
"time"

"github.com/golang/protobuf/proto"
"github.com/golang/protobuf/ptypes"
"github.com/google/go-cmp/cmp"
"google.golang.org/grpc/connectivity"
"google.golang.org/grpc/credentials"
"google.golang.org/grpc/internal/channelz"
"google.golang.org/grpc/internal/grpctest"
"google.golang.org/protobuf/encoding/prototext"
"google.golang.org/protobuf/protoadapt"
"google.golang.org/protobuf/reflect/protoreflect"
"google.golang.org/protobuf/reflect/protoregistry"
"google.golang.org/protobuf/runtime/protoimpl"
Copy link
Member

@dfawley dfawley Mar 26, 2024

Choose a reason for hiding this comment

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

WARNING: This package should only ever be imported by generated messages. The compatibility agreement covers nothing except for functionality needed to keep existing generated messages operational. Breakages that occur due to unauthorized usages of this package are not the author's responsibility.

I'd rather not use this package if it's at all possible to avoid it, unless you are extremely sure backward compatibility will be guaranteed for all our uses of it.

Copy link
Contributor Author

@Clement-Jean Clement-Jean Mar 27, 2024

Choose a reason for hiding this comment

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

yes, I agree that using protoimpl is half a solution. I'm figuring out how to imrpove this.

"google.golang.org/protobuf/testing/protocmp"
"google.golang.org/protobuf/types/known/anypb"
"google.golang.org/protobuf/types/known/timestamppb"

channelzpb "google.golang.org/grpc/channelz/grpc_channelz_v1"
Expand Down Expand Up @@ -98,13 +102,21 @@ type OtherSecurityValue struct {
RemoteCertificate []byte `protobuf:"bytes,2,opt,name=remote_certificate,json=remoteCertificate,proto3" json:"remote_certificate,omitempty"`
}

func (m *OtherSecurityValue) Reset() { *m = OtherSecurityValue{} }
func (m *OtherSecurityValue) String() string { return proto.CompactTextString(m) }
func (*OtherSecurityValue) ProtoMessage() {}
func (m *OtherSecurityValue) Reset() { *m = OtherSecurityValue{} }
func (m *OtherSecurityValue) String() string {
opt := prototext.MarshalOptions{Multiline: false}
return opt.Format(protoadapt.MessageV2Of(m))
}
func (*OtherSecurityValue) ProtoMessage() {}

func init() {
// Ad-hoc registering the proto type here to facilitate UnmarshalAny of OtherSecurityValue.
proto.RegisterType((*OtherSecurityValue)(nil), "grpc.credentials.OtherChannelzSecurityValue")
// Ad-hoc registering the proto type here to facilitate Unmarshal of OtherSecurityValue.
m := (*OtherSecurityValue)(nil)
s := "grpc.credentials.OtherChannelzSecurityValue"
mt := protoimpl.X.LegacyMessageTypeOf(m, protoreflect.FullName(s))
if err := protoregistry.GlobalTypes.RegisterMessage(mt); err != nil {
panic(err)
}
}

func (s) TestGetTopChannels(t *testing.T) {
Expand Down Expand Up @@ -270,7 +282,7 @@ func (s) TestGetServerSockets(t *testing.T) {
ids[2]: refNames[2],
}
if got := convertSocketRefSliceToMap(resp.GetSocketRef()); !cmp.Equal(got, want) {
t.Fatalf("GetServerSockets want: %#v, got: %#v (resp=%v)", want, got, proto.MarshalTextString(resp))
t.Fatalf("GetServerSockets want: %#v, got: %#v (resp=%v)", want, got, prototext.Format(resp))
}

for i := 0; i < 50; i++ {
Expand Down Expand Up @@ -620,11 +632,11 @@ func (s) TestGetSocket(t *testing.T) {
}), newSocket(czSocket{
security: &credentials.OtherChannelzSecurityValue{
Name: "YYYY",
Value: &OtherSecurityValue{LocalCertificate: []byte{1, 2, 3}, RemoteCertificate: []byte{4, 5, 6}},
Value: protoadapt.MessageV2Of(&OtherSecurityValue{LocalCertificate: []byte{1, 2, 3}, RemoteCertificate: []byte{4, 5, 6}}),
},
}),
}
otherSecVal, err := ptypes.MarshalAny(ss[6].Security.(*credentials.OtherChannelzSecurityValue).Value)
otherSecVal, err := anypb.New(ss[6].Security.(*credentials.OtherChannelzSecurityValue).Value)
if err != nil {
t.Fatal("Error marshalling proto:", err)
}
Expand Down Expand Up @@ -737,7 +749,7 @@ func (s) TestGetSocket(t *testing.T) {
for i := range ss {
resp, _ := svr.GetSocket(ctx, &channelzpb.GetSocketRequest{SocketId: skts[i].ID})
w := &channelzpb.Socket{}
if err := proto.UnmarshalText(want[i], w); err != nil {
if err := prototext.Unmarshal([]byte(want[i]), w); err != nil {
t.Fatalf("Error unmarshalling %q: %v", want[i], err)
}
if diff := cmp.Diff(resp.GetSocket(), w, protocmp.Transform()); diff != "" {
Expand All @@ -757,9 +769,9 @@ func escape(bs []byte) string {
func addr(a net.Addr) string {
switch a := a.(type) {
case *net.TCPAddr:
return string(a.IP)
return escape([]byte(a.IP))
case *net.IPAddr:
return string(a.IP)
return escape([]byte(a.IP))
}
return ""
}
4 changes: 2 additions & 2 deletions credentials/credentials.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ import (

"google.golang.org/grpc/attributes"
icredentials "google.golang.org/grpc/internal/credentials"
"google.golang.org/protobuf/protoadapt"
"google.golang.org/protobuf/proto"
)

// PerRPCCredentials defines the common interface for the credentials which need to
Expand Down Expand Up @@ -287,5 +287,5 @@ type ChannelzSecurityValue interface {
type OtherChannelzSecurityValue struct {
ChannelzSecurityValue
Name string
Value protoadapt.MessageV1
Copy link
Member

@dfawley dfawley Apr 3, 2024

Choose a reason for hiding this comment

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

This may be a breaking change. It is probably fine since it's experimental, and it's unlikely anyone is using it....so I think I'm OK with it. Just wanted to call it out.

Value proto.Message
}