feat: proto v2 support

[technicallyty]

Description

Closes: #10728

---

Author Checklist

All items are required. Please add a note to the item if the item is not applicable and
please add links to any relevant follow up issues.

I have...

• included the correct type prefix in the PR title
• added ! to the type prefix if API or client breaking change
• targeted the correct branch (see PR Targeting)
• provided a link to the relevant issue or specification
• followed the guidelines for building modules
• included the necessary unit and integration tests
• added a changelog entry to CHANGELOG.md
• included comments for documenting Go code
• updated the relevant documentation or specification
• reviewed "Files changed" and left comments if necessary
• confirmed all CI checks have passed

Reviewers Checklist

All items are required. Please add a note if the item is not applicable and please add
your handle next to the items reviewed if you only reviewed selected items.

I have...

• confirmed the correct type prefix in the PR title
• confirmed ! in the type prefix if API or client breaking change
• confirmed all author checklist items have been addressed
• reviewed state machine logic
• reviewed API design and naming
• reviewed documentation is accurate
• reviewed tests and test coverage
• manually tested (if applicable)

[lgtm-com]

This pull request introduces 2 alerts when merging 93afb5f into 914af60 - view on LGTM.com

new alerts:

• 1 for Useless assignment to local variable
• 1 for Duplicate 'if' branches

[aaronc]

Let's change all the Marshal/Unmarshal methods to take proto.Message rather than interface{}

[fdymylja]

preliminary review - with @aaronc, we discussed to still keep the old proto.Message interface (the one in gogoproto) because our protov2 types implement that, and then try to promote a legacy proto.Message to the new proto.Message interface (the one with protoreflect)

[fdymylja]

nvm didn't see aaron already pointed that out! :D

[technicallyty]

having some trouble with marshaling an interface with golang/protobuf jsonpb marshaler.

the gogo version doesn't require a resolver and can marshal interfaces without it. The golang/protobuf needs a resolver, else it will fall back to the global registry, which is not used here.

the issue is fixed when using a resolver in the golang jsonpb marshaler, but our current interface registry seems tightly coupled with gogo's message interface, and changing it to golang's message interface seems to open a can of worms.. (it breaks our gateway code which is tied to gogo, and some scatterings of our codec).

any ideas on what to do here? @aaronc @fdymylja

see test showing the above here: d803846#diff-0f7a2f9aad8a3eb869cf813e0a06fcc45bea149d0d5480b0fb80ca6a73e1c6c5R209-R244

[lgtm-com]

This pull request introduces 2 alerts when merging 152faf0 into b67c4b3 - view on LGTM.com

new alerts:

• 1 for Useless assignment to local variable
• 1 for Duplicate 'if' branches

[aaronc]

having some trouble with marshaling an interface with golang/protobuf jsonpb marshaler.

the gogo version doesn't require a resolver and can marshal interfaces without it. The golang/protobuf needs a resolver, else it will fall back to the global registry, which is not used here.

so is the issue primarily when an interface registry isn't explicitly specified?

the issue is fixed when using a resolver in the golang jsonpb marshaler, but our current interface registry seems tightly coupled with gogo's message interface, and changing it to golang's message interface seems to open a can of worms.. (it breaks our gateway code which is tied to gogo, and some scatterings of our codec).

how does it break the gateway code? proto.Message should be the same with both golang and gogo

[lgtm-com]

This pull request introduces 2 alerts when merging 62abf15 into fd5b97b - view on LGTM.com

new alerts:

• 2 for Useless assignment to local variable

[technicallyty]

had to add this in, for some reason one of the JSON functions down the line is injecting "null" for empty Any slices ?? any idea if theres something better i can do here? @aaronc @fdymylja

[lgtm-com]

This pull request introduces 2 alerts when merging 4a4ef5b into 6ea2049 - view on LGTM.com

new alerts:

• 2 for Useless assignment to local variable

[aaronc]

@technicallyty how close was this to being ready? I wonder if we should try to push it through for 0.46

[technicallyty]

@technicallyty how close was this to being ready? I wonder if we should try to push it through for 0.46

last i remember was being pretty stuck and not sure where to go next. i can try picking this up again but iirc wasn't 0.46 already delayed? not sure if we want to introduce something that could delay it again 🤔

[aaronc]

@technicallyty how close was this to being ready? I wonder if we should try to push it through for 0.46

last i remember was being pretty stuck and not sure where to go next. i can try picking this up again but iirc wasn't 0.46 already delayed? not sure if we want to introduce something that could delay it again 🤔

Okay, let's postpone. If it was already working that would be a different story

[github-actions]

This pull request has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.