[encoding] add methods to expose original headers

[AllenLuUber]

We already made similar change in #1426 to store those original-case headers and plumbed into tchannel's implementation.

For TChannel(header is case-sensitive) proxying case working directly with tchannel-go, users need a way to access all the headers with its original case key. This PR adds a method in Call to users gets the baggage. We allocate a new map so the users can't mutate this state.

[codecov[bot]]

Codecov Report

Merging #2108 (d8a2106) into dev (90b9cd1) will decrease coverage by 0.01%. The diff coverage is 100.00%.

@@            Coverage Diff             @@
##              dev    #2108      +/-   ##
==========================================
- Coverage   87.65%   87.63%   -0.02%     
==========================================
  Files         249      249              
  Lines       13944    13954      +10     
==========================================
+ Hits        12222    12229       +7     
- Misses       1333     1335       +2     
- Partials      389      390       +1     

Impacted Files	Coverage Δ
api/encoding/call.go	100.00% <100.00%> (ø)
call.go	100.00% <100.00%> (ø)
transport/tchannel/peer.go	93.42% <0.00%> (-3.95%)	:arrow_down:

---

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update 90b9cd1...d8a2106. Read the comment docs.

[bryndin]

Headers has canonical and the original headers. Canonical are accessed viaGet(k string) (string, bool), original are via OriginalItems() map[string]string.

Why having such a different interface? Instead we can do Get(...) and GetOriginal(...) to keep them homogeneous.

Then in api/encoding/call.go to the existing Header(k string) string we can add a homogeneous OriginalHeader(k string) string, instead of OriginalItems().

Extra benefit, no need to copy the whole map as it's done in OriginalItems().

[AllenLuUber]

I don't like the current Headers method. I don't see the use case of getting the full list of header keys without the values. In reality, we have code that loops through the Headers() and Get value of each key. Instead of letting users to do that, we simply return the map to the user. I think the reasonable API is Get(key string) and Items() map[string]string

Also note we care for the original header keys. I don't think the Get and GetOriginal(...) will meet the use case here. Extra benefits is overstated. We still need to allocate a slice to return []string if we return OriginalHeaders for the keys, of the same size of the current map.

[gopalchouhan]

@bryndin Get(k string) (string, bool) is available in transportRequest and that is unaccessible from the Call object. We still need an interface in Call to get the headers.

@AllenLuUber I am ok with adding this method to return the headers. Any particular reason for not naming this method as Headers()?

[AllenLuUber]

@AllenLuUber I am ok with adding this method to return the headers. Any particular reason for not naming this method as Headers()?

No :) I just inherited the name from header's API.

[bryndin]

TL;DR it's a good change. Can we unify APIs further?

I see. So the main use case is to give users all headers (Items/OriginalItems), not to expect them to know the particular key (Get).

I don't like the current Headers method. I don't see the use case of getting the full list of header keys without the values. In reality, we have code that loops through the Headers() and Get value of each key.

True. My point, having different APIs in api/encoding/call.go and in api/transport/header.go is not user friendly.

header.go provides Items() and OriginalItems(), in addition Get(key) that does canonization of key.

call.go does

• Header(key) to match Get(key)
• you're adding OriginalHeaders() to match OriginalItems()
• do we need Headers() to match Items() as well?
• should we deprecate HeaderNames() and point users to Headers()/OriginalHeaders()?
• Header() docstring needs to explain "canonization", as it won't distinguish b/w "Foo" and "foo". Without it the need of Header() over Items() is not clear without digging into Get() of headers.