opentracing-java
opentracing-java copied to clipboard
opentracing
TextMap and get(String key)
The Tracer APIs demonstrate using TextMap as the carrier when extracting tracing information from HTTP headers [1]. However the TextMap API lacks a get(String) type API, and instead provides an iterator. This makes the cost of extracting values from a TextMap proportional to the number of headers and requires iterator creation for each extraction. Can someone explain the rational for the lack of a get(..) API on TextMap, and is there an alternative approach/carrier to use which does provide this type of API?
[1] https://github.com/opentracing/opentracing-java/blob/master/opentracing-api/src/main/java/io/opentracing/Tracer.java#L78
If I remember correctly, there were two reasons:
- some transports may not give by-key access to the headers (because of streaming, what not)
- in case of HTTP, there's usually no way of telling how the headers are modified, some frameworks might present them as lowercase, others in uppercase, others in First-Cap-Before-Dash.
some transports may not give by-key access to the headers (because of streaming, what not)
In case of streaming is the thought that the iterator will block until more headers are present? In my experience it is not common to stream the meta data portion of requests (e.g. headers) because this may dictate how the body is processed.
in case of HTTP, there's usually no way of telling how the headers are modified, some frameworks might present them as lowercase, others in uppercase, others in First-Cap-Before-Dash.
I see. That would be the responsibility for TextMap implementations if they must preserve HTTP semantics. For example Netty's HttpHeaders provides a Map API with get(..) that accounts for case insensitive compares. The method in TextMap could even be default:
@Nullable
default String get(CharSequence name) {
// use iterator to find the value, this method can be overridden if comparison or lookup can be improved.
}
I just remembered another reason. Some tracers encode baggage in http headers as {some-prefix}-{baggage-key}: {baggage-value | urlencode}. The benefits of this format are (a) it's easily readable on the wire / tcpdump, (b) it imposes less limitations on the length of baggage as opposed to encoding everything into one header. However, when deserializing this back from HTTP headers there is no way for the tracer to know which keys to as for from the Headers map, it can only do it by iterating and checking the prefix.
In retrospect, the same could've been achieved via different encoding, e.g.
{some-prefix}-0: {number of baggage items}; {baggage-key-0}={baggage-value-0}
{some-prefix}-n: {baggage-key-n}={baggage-value-n}
That encoding would've also addressed the problem of the keys changing case in some http frameworks.
Given that changing the text map to provide direct get/set methods would be backwards incompatible change, perhaps the alternative is to allow the carriers support an additional interface with direct get method, so that tracers that are able to resolve their context via get could check for that interface and use it, otherwise fallback to iterating.
Given that changing the text map to provide direct get/set methods would be backwards incompatible change
Is this true? As suggested in https://github.com/opentracing/opentracing-java/issues/169#issuecomment-324694434 it could be done with a default method.
@scottmitch we are restricted to Java 1.6 features due to compatibility issues with Android (and other legacy stuff); I believe that prevents us from pursuing the approach above.
@Scottmitch are you trying to integrate Netty with OpenTracing? Are you able to share the code?
Going to mark this issue as "Enhancement" as it looks we need Java 1.7.x and above. Is this the case? @yurishkuro
Only if we did this via default method. An optional interface would work with 1.6.
The other thing is - only one person has ever asked for this. I think rule of three should apply.