uber / tchannel

network multiplexing and framing protocol for RPC
MIT License
1.15k stars 129 forks source link

docs/protocol: define headers for tchannel language and version #1398

Closed jcorbin closed 8 years ago

jcorbin commented 8 years ago

This defines three new init headers:

r @breerly @prashantv @mranney @ShanniLi

jcorbin commented 8 years ago

The plan is that a central authority, such as hyperbahn, can collect and report on these things... and eventually enforce cut off versions.

ShanniLi commented 8 years ago

:+1:

ShanniLi commented 8 years ago

@jcorbin So, if Hyperbahn decides to cut off a version. What init res message should it send?

prashantv commented 8 years ago

lgtm

jcorbin commented 8 years ago

@ShanniLi probably:

Basically, an even harder form of our current draining mechanism.

jcorbin commented 8 years ago

Oh furthermore @ShanniLi, these fields would be required of both init req and init res, since when hyperbahn connects to you, it cares about the versions sent in your init res to its init req.

ShanniLi commented 8 years ago

@jcorbin Thanks, makes sense. :)

willsalz commented 8 years ago

Why are these needed at all? Why does tchannel/hyperbahn care about implementation language? And what is tchannel_language_version? Is that a SemVer of the library implementing hyperbahn/tchannel? Are you planning on cutting off older versions of libraries? That seems crazy.

jcorbin commented 8 years ago

Great questions @willsalz , we're adding this primarily to get visibility in production so that we can help eliminate version skew. It may be possible to get similar benefit by auditing repositories and build artifacts internally, but the reality of what's being used in production is likely to be much more reliable than any such integration.

the tchannel_language_version is the version of "language", whatever that means for the language:

Eventually we may decide to cut off old versions, but only based on clear auditing of what versions are being used and weighed against whatever the reason is for that enforcement.

We'll first have to track getting everyone upgraded to libraries which send this new field, including e.g. a backported version to tchannel-node's 2.x series.

HelloGrayson commented 8 years ago

:ship:

willsalz commented 8 years ago

OK why not have people put this in headers or something?

jcorbin commented 8 years ago

This is in the headers, of the connection init message @willsalz ; we'd like to avoid transmitting it as overhead on every single call message.