Support THeader protocol #564
Conversation
THeader is an optional wrapper for Thrift RPC messages that allows for requests to include out-of-band headers, useful for things like distributed tracing in service oriented architectures. This change only implements THeader support on the server side. We can auto-detect the protocol on a per-request basis based on the first two bytes. THeader starts with 0x0FFF, whereas binary starts with 0x8001.
pguillory
requested review from
fishcakez,
jparise and
scohen
as code owners
lib/thrift/protocol/theader.ex
Outdated
| # Try other protocols. | ||
| def deserialize(payload) do | ||
| with {:ok, message} <- Binary.deserialize(:message_begin, payload) do | ||
| headers = %{protocol: Binary} | ||
| {:ok, headers, message} | ||
| end | ||
| end |
There was a problem hiding this comment.
I'm not sure it feels quite right for the THeader module to be the thing that defers to the other protocols (like Binary). I think I would have expected the protocol handler to attempt to match on the magic header one level up and then invoke the appropriate protocol-specific deserialization calls from there.
There was a problem hiding this comment.
THeader is really a Transport and not a Protocol. Likely we want to move the abstraction to that level.
There was a problem hiding this comment.
@jparise: With its protocol_id field, THeader knows about selecting between binary/compact protocols. So to me it feels like a natural place also to be doing protocol auto-detection.
@fishcakez: True that it's a transport. I'll rename it to Thrift.Transport.THeader.
lib/thrift/protocol/theader.ex
Outdated
| # THeader format: | ||
| # https://github.com/apache/thrift/blob/master/doc/specs/HeaderFormat.md |
There was a problem hiding this comment.
Maybe this should be part of a moduledoc block?
This link is also duplicated below (above deserialize).
|
If the server receives a THeader call, it needs to return a THeader reply, otherwise some clients may break. Generally speaking clients will have a configured a transport/protocol and always use it. |
Will work on this. |
THeader is an optional wrapper for Thrift RPC messages that allows for requests
to include out-of-band headers, useful for things like distributed tracing in
service oriented architectures.
This change only implements THeader support on the server side. We can
auto-detect the protocol on a per-request basis based on the first two bytes.
THeader starts with 0x0FFF, whereas binary starts with 0x8001.