Any plans to migrate off of gogo protobuf?

[GiedriusS]

Hello from the Thanos project 👋

We've been working on trying to migrate to protobuf v2 API from the unmaintained gogoprotobuf. See this issue for details. Since Thanos calls Cortex code, it means that we need to be synchronized on this. Currently, my work is blocked due to this:

panic: protobuf tag not enough fields in ThanosLabelsResponse.state: 

goroutine 432 [running]:
github.com/gogo/protobuf/proto.(*unmarshalInfo).computeUnmarshalInfo(0xc000159d60)
	/home/circleci/go/pkg/mod/github.com/gogo/[email protected]/proto/table_unmarshal.go:341 +0x138a
github.com/gogo/protobuf/proto.(*unmarshalInfo).unmarshal(0xc000159d60, {0x1163c40}, {0xc0004699b0, 0x18, 0x18})
	/home/circleci/go/pkg/mod/github.com/gogo/[email protected]/proto/table_unmarshal.go:138 +0x67
github.com/gogo/protobuf/proto.(*InternalMessageInfo).Unmarshal(0xc0000f9840, {0x14814c8, 0xc0007675f0}, {0xc0004699b0, 0x18, 0x18})
	/home/circleci/go/pkg/mod/github.com/gogo/[email protected]/proto/table_unmarshal.go:63 +0xd0
github.com/gogo/protobuf/proto.(*Buffer).Unmarshal(0xc00002b018, {0x14814c8, 0xc0007675f0})
	/home/circleci/go/pkg/mod/github.com/gogo/[email protected]/proto/decode.go:424 +0x153
github.com/gogo/protobuf/proto.Unmarshal({0xc0004699b0, 0x18, 0x18}, {0x14814c8, 0xc0007675f0})
	/home/circleci/go/pkg/mod/github.com/gogo/[email protected]/proto/decode.go:342 +0xef
github.com/gogo/protobuf/types.UnmarshalAny(0xc0001e5680, {0x14814c8, 0xc0007675f0})
	/home/circleci/go/pkg/mod/github.com/gogo/[email protected]/types/any.go:127 +0x1d8
github.com/cortexproject/cortex/pkg/querier/queryrange.(*Extent).toResponse(0xc00002b1e0)
...

This is because of the problem described here. I think it's impossible workaround this from Thanos side without actually migrating Cortex to v2 protobuf API because Cortex at the moment of writing uses anypb, and that calls the code which looks for those fields that don't exist in the newly generated code. Perhaps it would be possible to fix this problem from Thanos side somehow but there is one critical problem - the newer state struct member is not recognized by the gogo code which means that it will always panic.

I hope this ticket contains enough data. Has the Cortex team ever thought about this? It's tempting to update our fork of Cortex and to do (attempt to do) the migration there.

[alvinlin123]

I think migrating to an actively maintained lib is always a good idea. I am not sure about the migration effort though since I haven't been working with Go ecosystem for long. @GiedriusS would this be something one can do in few hours? few days, or few weeks?

[GiedriusS]

Not sure about how much effort is needed for Cortex but I have done this recently for the Thanos codebase so it shouldn't take too long. Glad to hear that you would be open to such changes. I'll try migrating and see 👍

[alvinlin123]

@GiedriusS based on thanos-io/thanos#5504 it looks like Thanos moved away from depending on Cortex; in this case, this issue is not blocking your work on thanos-io/thanos#5421 anymore right?

[stale]

This issue has been automatically marked as stale because it has not had any activity in the past 60 days. It will be closed in 15 days if no further activity occurs. Thank you for your contributions.

[GiedriusS]

Not stale

[alanprot]

Hi @GiedriusS :D

How is the thanos migration off gogo? I think both projects should try to move out of it before its even harder.. WDYT?

I just worried if we will have some perf penalty.

[stale]

This issue has been automatically marked as stale because it has not had any activity in the past 60 days. It will be closed in 15 days if no further activity occurs. Thank you for your contributions.

[alanprot]

/remove stale