opentracing
Add get to TextMap
I complained about this a couple years ago, but re-complaining as it seems possible some things can change now.
TextMap is accidentally constrained to an awkward pattern where you can add keys by name, but not retrieve by name. This is awkward because getting by name involves creating an iterator then scanning it. It also encourages people to cast to vendor types like this for routine functionality.
I got bored looking to find a single implementation of TextMap that doesn't support get by name natively or that doesn't instantiate an intermediate java Map due to the awkwardness! The net result is guaranteed less efficient operations for the most common propagation use cases.
Can we please fix this as it will remove a lot of spaghetti. The fix is to add @Nullable String get(String key)
in many cases the native representation of the headers is a multi-map. Did you mean List<String> get(String key)? Or will the adapter have to concatenate into one string?
The current method Iterator<Map.Entry<String, String>> iterator(); also suggests that multi-values are not expected (although technically, there could be multiple entries with the same key). I'm not sure if those would be needed for the tracing use-case. I'd say, the adapter should get the first header. If you think there is a use case to get the list of headers, I'd add two methods, so that no objects have to be allocated for the common case:
-
@Nullable String getFirst(String key) -
List<String> get(String key)
Otherwise, big +1. Seems like a no-brainer to me.
The W3C tracestate header is by definition a multi-value header. Http protocol does not guarantee how those are transmitted: could be as repeated headers, could be as a comma-separated single header.
The W3C tracestate header is by definition a multi-value header.
So adding the getFirst and get/getAll methods makes even more sense, it seems?
I realize that the header definition is multi-map, but I don't know of a good case for TextMap to support that format. @yurishkuro do you have any examples where that is important?
As I mentioned above, W3C tracestate header is explicitly multi-value, so even if the sender puts all values into a single header the routing proxies are within their rights to rewrite that as one value per header. The currently on-hold correlation-context header will also be multi-value.
Another example, Jaeger supports baggage propagation using the header jaeger-baggage: k1=v1, k2=v2, which is also subject to the same relaxed rules in HTTP spec, i.e. it can be sent as
jaeger-baggage: k1=v1
jaeger-baggage: k2=v2
Of course, we can say that the TextMap itself never needs the multi-map semantics, and when we encounter the above use cases it's the job of the carrier to merge the entries and return them as comma-separated string (100% valid behavior per HTTP spec). However, there is some inherent inefficiency in doing that (string concatenation, only to be parsed again immediately), and the whole (and only, imho) point of this ticket is to improve efficiency of reading the values without instantiating an iterator.
@yurishkuro Thanks for the details. In that case, do you see any problem with the API @felixbarny proposed?