Support additional validation for keys and values in baggage builder

[jimshowalter]

We need to restrict the keys and values in our baggage, to avoid having it become an untyped free-for-all dumping ground, and to keep it from exceeding the size limits).

We were hoping to do that by subclassing ImmutableBaggage.Builder and overriding the put method.

Unfortunately, we can't see private final List data;, so when we try to override the put method, it won't compile.

An easy fix would be to provide a protected accessor, similar to the one in ArrayBackedBaggage, but it might be better to provide an overload of the put method that has parameters for a BaggageKeyValidator and a BaggageValueValidator.

[jimshowalter]

See also #2200

[jkwatson]

Hi @jimshowalter . Baggage is an interface. Can you provide your own implementation of it to do this?

[jimshowalter]

There's a lot of valuable code in the existing implementation that it would be a shame to fork.

I wound up implementing it by composition, but it's still suboptimal, having to override more than the put method.

An overload that takes validators seems like a good solution. In order to validate pairs, there should just be one validator, and the validate method should take two arguments.

The existing validation could be in a base class.

[anuraaga]

@jimshowalter Do you think you could file an issue in https://github.com/open-telemetry/opentelemetry-specification for this functionality? It seems useful in general so we should think it out so we can apply the pattern to all the languages.

[jkwatson]

Just so we can try to make the best decisions about API additions: can you explain how you want to use this feature? Are you writing your own propagator? If so, could the logic for the additional validations just go into the propagator implementation itself?

[jimshowalter]

We're trying to use as much of opentelemetry out of the box as possible.

But with hundreds of microservices, if everybody adds completely ungoverned keys and values to the baggage, two (or more) bad things happen.

First, we risk running out of space.

Second, and worse, fields we expected to be passed from one link in the chain to the next are lost.

We need to schematicize the baggage, and turn it into a contract.

We centralized a simple text file of key names to value regexes, and wrote some simple validation code that fetches the file at startup, and uses that to constrain what can be put in the baggage.

All of which was entirely doable, it's just that where we want to simply subclass the ImmutableBaggage.Builder and overload a single method (put) in order to call our added validation, we instead had to create a builder class that wraps the current builder.

If the current builder accepted a validator, we could just delegate in our instance:

interface BaggageValidator {
void validate(String key, String value);
}

public interface BaggageBuilder {

BaggageBuilder put(String key, String value, BaggageEntryMetadata entryMetadata, BaggageValidator validator);
BaggageBuilder put(String key, String value, BaggageEntryMetadata entryMetadata);

[jimshowalter]

Filed open-telemetry/opentelemetry-specification#1490.