Envoy: Feature Request: add support for various Thrift protocols

[9len]

This is a general issue for adding support for various dialects of Thrift:

• Apache Thrift
• Fbthrift
• Finagle Thrift/TTwitter

Ideally we would have some visibility into methods called and exceptions thrown, similar to URIs and response codes in HTTP.

@bierbaum has volunteered to work on this on Turbine Labs' behalf, and will follow up here with a design proposal.

[zuercher]

I wrote a design document for this. Would love comments from anyone interested in this.

https://docs.google.com/document/d/1QuiZMT1ruetavVHsj4Sv55YpBFVxTEzJj-ZpOJqAboc/edit?usp=sharing

[mattklein123]

@zuercher sweet will take a look. Can we also make sure to get some folks with Thrift domain knowledge to review? Can you recommend anyone specific?

[brian-pane]

Thanks @zuercher, I've shared the doc with the people at Pinterest who are working on Thrift.

[zuercher]

I've made some updates to the doc with feedback I received outside of Github and Google docs.

@mattklein123 what's the process for going forward once I finish resolving the remaining comments on the doc?

I'd also like your take on the feasibility of writing a base L7 router that could be reused for multiple protocols (per my response to @htuch's question about keeping parity with GRPC stats/tracing/logging).

[mattklein123]

@zuercher I would just go for it. It's on my list of things to review. I will take a look tomorrow morning.

[mattklein123]

@zuercher the plan LGTM. I dropped some comments in re: @htuch comments. We can continue the discussion there or here, whichever is easiest.

[zuercher]

Per the extensions policy, I'm going to use this issue as the extension proposal.

I'll be the maintainer-sponsor for the extension and do at least the initial work on it, but I need another contributor to participate in code reviews. Is there someone watching this ticket who's willing to volunteer to fill that role?

[brian-pane]

I volunteer to participate in code reviews.

[mattklein123]

Thank you @zuercher and @brian-pane! SGTM.

[agile6v]

@mattklein123 Which version of envoy will support the thrift? Thanks.

[stale[bot]]

This issue has been automatically marked as stale because it has not had activity in the last 30 days. It will be closed in the next 7 days unless it is tagged "help wanted" or other activity occurs. Thank you for your contributions.

[9len]

@zuercher has been making incremental progress towards this. C.F.:

• 2c379306c272e9cc4f79e34de3df41883e19c875
• a89eb3c59204caa3e0cd1bcd845bcb1a0be95267
• e6a65cc6348065b863be8eede1639caec34f91dd
• 4521e89da33ce44ebdc61b2b1da07712341d743d
• 56a047b5064bba8108ec31c2b6a1f94da1711c20

[agile6v]

Hello

@9len @mattklein123 @zuercher

I want to know what features thrift will support in enovy？

E.g. "Circuit Breaker"、"Fault Injection"、"Dynamic Router"、"Request Mirror"、"Rate Limiting" etc

Regards.

[zuercher]

Dynamic Router meaning routing based on request properties? Yes. A trivial version is currently checked in.

Circuit breaking and rate limiting are on my roadmap, but I don't have a timeline.

Fault Injection will require a custom filter. I don't have that on my roadmap, but it should be possible. Request mirroring also isn't on my roadmap, but it is also a feasible feature.

[agile6v]

Great job. We are all looking forward to these features.

[stale[bot]]

This issue has been automatically marked as stale because it has not had activity in the last 30 days. It will be closed in the next 7 days unless it is tagged "help wanted" or other activity occurs. Thank you for your contributions.

[brian-pane]

(Updating to keep the stale-bot from closing.)
This feature is still under active development.

[AceHack]

So excited for this!!

[AceHack]

May I ask how do you decode thrift without the IDL if it's not a self describing protocol?

[zuercher]

Envoy mostly doesn't need to know about the details of the message. It's a thrift struct, which has fields (which my contain nested structs) but eventually an end-of-struct indicator appears and that's the end of the message. For stats, Envoy needs to know that reply messages are structs where the presence of field id 0 means "success" and any other field id is an exception (of a type opaque to Envoy). For generating protocol-level exceptions, Envoy needs to know the format of the Application Exception struct. The Twitter protocol variant has some well-defined structs in it as well.

[jbye]

Looking forward to this! Are there any progress updates?

[mattklein123]

@zuercher @brian-pane what's the status of Thrift? Can we close this out as fixed and then track additional feature requests as new issues?

[brian-pane]

The Thrift routing implementation is in place (thanks to @zuercher). I haven't yet had a chance to do much interoperability testing, but that should happen over the next couple of months. +1 for closing this and tracking new features and any bugs as new issues.

https://github.com/envoyproxy/envoy/tree/master/source/extensions/filters/network/thrift_proxy

[mattklein123]

Cool, closing as fixed.