Add keys func to TextMapCarrier

[codeboten]

As per spec, the carrier must provide a keys method

Keys

The Keys function MUST return the list of all the keys in the carrier.

https://github.com/open-telemetry/opentelemetry-specification/blob/master/specification/context/api-propagators.md#keys

[Aneurysm9]

I don't read the spec as requiring that the carrier must provide a Keys method, but rather than a Propagator may optionally accept a Getter:

Optional arguments:

• A Getter invoked for each propagation key to get. This is an additional argument that languages are free to define to help extract data from the carrier.

This Getter is undefined in terms of structure and behavior, though a possible solution is defined in the form of a class with Get and Keys methods.

One of the ways to implement it is Getter class with Get and Keys methods as described below. Languages may decide on alternative implementations and expose corresponding methods as delegates or other ways.

Indeed, it seems that the expectation is that the Getter and Setter implementations would be independent of the carrier itself:

Getter and Setter are optional helper components used for extraction and injection respectively, and are defined as separate objects from the carrier to avoid runtime allocations, by removing the need for additional interface-implementing-objects wrapping the carrier in order to access its contents.

Getter and Setter MUST be stateless and allowed to be saved as constants, in order to effectively avoid runtime allocations.

We could add a Keys method to the TextMapCarrier interface, though I'm not sure that's any more or less conformant to the spec than where we're at now or finding some other mechanism for enumerating values in a carrier.

[punya]

Reading the spec alongside the Java-language reference implementation, I see that Getter and Setter are defined separately from the carrier, but are expected to be totally coupled to it. So, for example, a carrier defined as a Map<String, String> would have an associated getter and setter that can do things with that representation. The idea behind keeping the getter/setter stuff separately was to avoid the need to wrap the Map in a whole new object, thus saving allocations in a critical path.

This design makes sense in Java because the carrier is a generic argument:

// getter for carriers of type C
interface Getter<C> {
  Iterable<String> keys(C carrier)
  String get(C carrier, String key)
}

interface TextMapPropagator {
  // Extract from context given a carrier and a *compatible* getter.
  <C> Context extract(Context ctx, C carrier, Getter<C> getter);
}

However, this design doesn't make sense in pre-generics Go (I would argue that it will remain un-idiomatic even once Go has generics), because in Go we specialize TextMapCarrier to a single interface type (i.e. we're already committed to the extra allocation that Java is trying to avoid). For a given getter to use private knowledge of its associated carrier type, it would need to do a runtime type assertion.

Instead, I think it's better to do as the original issue suggested and simply add a Keys method to the TextMapCarrier interface, allowing propagators to enumerate and filter keys.

[punya]

Digging a bit further - if I make Keys() a required method of TextMapCarriers, then http.Header no longer implements TextMapCarrier. It's easy enough to create a wrapper to satisfy the interface, but it's less ergonomic than what we had before.

// before
prop.Extract(ctx, req.Header)

// after
prop.Extract(ctx, HeaderCarrier(req.Header))

I'm going to submit a PR with this design, but it's worth thinking about alternatives.