rpc: install custom proto codec

[stevendanna]

This change enables future upgrades of google.golang.org/grpc.

Recent changes to google.golang.org/grpc have changed the default
proto codec such that it is incompatible with gogoproto:

grpc/grpc-go#3958

While proto.Marshal does do a check for the Marshal/Unmarshal it
appears to do this through reflection methods that will panic on
fields using gogoproto.nullable.

To avoid this, we install a proto codec that does an interface check
before dispatching to proto.Marshal/proto.Unmarshal.

Note that I haven't replicated the protobuffer caching that previously
existed in google.golang.org/grpc. The referenced change claims
removing the caching may be a small performance win; however, they
also removed the typecheck that we need to do, so it is unclear.

Fixes #60531

Release note: None

[cockroach-teamcity]

This change is 

[stevendanna]

I've left out a release justification as I imagine this can wait until we've cut the release branch.

[stevendanna]

@pbardea I believe you also need something like this. Perhaps now that master is reopen we can merge the codec separately from the dep updates so that those can be considered in isolation.

I believe @knz and @tbg said the codec changes looked good in #60709; however, I had to make a couple changes to account for some cases that our tests didn't seem to hit.

[pbardea]

LGTM with nits given the comments in 60709

Reviewable status: complete! 0 of 0 LGTMs obtained (waiting on @stevendanna)

---

pkg/rpc/codec.go, line 1 at r1 (raw file):

// Copyright 2020 The Cockroach Authors.

Should this be 2021? (And the test)

---

pkg/rpc/codec_test.go, line 37 at r1 (raw file):

		{"roachpb.GetRequest", func() interface{} { return &roachpb.GetRequest{} }},
	} {
		t.Run(fmt.Sprintf("can call marshal and unmarhsal on %s", test.name), func(t *testing.T) {

nit: typo in unmarhsal, i also have a slight preference for test names without spaces to avoid needing to manually convert when debugging. Maybe something like marshal-unmarshal-%s or TestCodedMarshalUnmarshal and just test.name here? Don't feel strongly here, take it or leave it :)

---

pkg/testutils/lint/lint_test.go, line 1654 at r1 (raw file):

				// Using deprecated WireLength call.
				stream.GrepNot(`pkg/rpc/stats_handler.go:.*v.WireLength is deprecated: This field is never set.*`),
				// rpc/codec.go imports the same proto package that grpc-go imports (as of crdb@dd87d1145 and grpc-go@7b167fd6)

super-nit: . at end of line

[knz]

@erikgrinaker can you review this too please?

[erikgrinaker]

Reviewed 2 of 5 files at r1, 2 of 3 files at r2.
Reviewable status: complete! 1 of 0 LGTMs obtained (waiting on @stevendanna)

---

pkg/rpc/codec_test.go, line 40 at r2 (raw file):

err = testCodec.Unmarshal(marshaled, test.msgBuilder())
			require.NoError(t, err, "unmarshal failed")

Maybe we should check that the unmarshaled message equals the marshaled one (using proto.Equal) and put some contents in the messages too?

[stevendanna]

Maybe we should check that the unmarshaled message equals the marshaled one (using proto.Equal) and put some contents in the messages too?

I've added a basic version of this but didn't add too many populated fields. Unfortunately, most methods of comparing the Go representation of the proto are broken for cases where you might be passing in different types of protos. For now, I've done some ugly things with reflect to make DeepEqual work for the proto's used in this test.

[erikgrinaker]

Yeah, comparing the Go structs using e.g. DeepEqual is a hassle because there's lots of Protobuf internals. You can just use proto.Equal() to compare the fields we care about and ignore the internals.

[stevendanna]

You can just use proto.Equal() to compare the fields we care about and ignore the internals.

Unfortunately, I don't think we can. Google's proto.Equal() does not work correctly for gogoproto generated types and Gogoproto's proto.Equal doesn't work correctly because of some bugs.

[erikgrinaker]

Groan, you're right. Thanks for going the extra mile here!

[stevendanna]

bors r=[erikgrinaker,pbardea]

[craig]

Build failed (retrying...):

• GitHub CI (Cockroach)

[craig]

Build succeeded:

• GitHub CI (Cockroach)