automagic application header propagation

blampe commented 9 years ago

@angiez00 encountered a bug where the developer had assumed application headers would be forwarded to all downstream calls, when in reality they are not.

The expectation here is

You get an inbound HTTP request with some headers. Some of those headers always (or almost always) need to be included in downstream tchannel requests (as application headers).
You get an inbound tchannel request. Same story. Some of those request headers always need to propagated downstream.

The obvious use cases here are authn/z.

I don't think we can blindly forward all headers, that's too error-prone and probably overkill. Instead I think we can give developers a way to configure tchannel to always forward the provided list of headers. We can add hooks for HTTP if there's a strong need for that.

@abhinav @breerly @junchaowu

cc @Raynos @prashantv for perspectives from other languages. AFAIK node/go have context objects that get passed everywhere, and you could copy the request headers to that object for downstream calls?

jc-fireball commented 9 years ago

Some people may like it and some people may not like it. They can hold that headers things to all the sub calls.

prashantv commented 9 years ago

Go will propagate the headers right now if they use the same context. If they want to override them, they can get a sub context using WithHeader.

I added this as a note to the guide since it may surprise people (I actually forgot about this behaviour).

Raynos commented 9 years ago

:-1: on automatic header propagation.

We could allow a per sub channel whitelist of headers to copy. I.e. For each service I talk to; let me tell tchannel which incoming headers to copy.

abhinav commented 9 years ago

I'm in favor of users manually copying the headers downstream. That's the least surprising behavior.

simongalkov commented 9 years ago

What about throwing a warning if the "suggested" headers are not being sent? If the service doesn't want to send them, the service can set those specific headers as None and the warning would go away.

blampe commented 9 years ago

People tend to ignore warnings ;)

+1 to whitelisted propagation.

uber / tchannel-python

automagic application header propagation #214