Another approach would be to have an engine-level configuration option that allows you to specify authorities for which you know you can force h2. In some ways I like that better than a per-request approach, since support for this functionality is non-standard and relatively uncommon, and it avoids inadvertently dispatching requests to destinations that don't support it. It's more a property of the destination (:authority) than it is of the request.

That said, I'm okay with the per-request approach as well (it just means the application will have to implement its own whitelist).

@goaway thanks for all the comments. I am answering to all of them in aggregate because I think they are all related.

I had originally written this PR with that approach, and actually with something similar to what you suggest below re: central configuration.

However, I moved to this approach for a few reasons:

1. The enum allows us to express potential expansion of future options: h3, auto (when alpn support lands on upstream connections).
2. The enum allows us to more easily migrate existing code to those future options without breaking the API if we find ourselves in a future where we care more strictly about API breakage as, hopefully, we see more adopters.
3. IMO the protocol support of your destination is an application level concern (if the library can't do protocol negotiation for you), and the application developer should tell us that prior knowledge. I am also not a big fan of centralized configuration given at boot time, as you know :)

Right now absence of the header defaults us to http1 per the implementation in the Http::Dispatcher::setDestinationCluster. I can do:

I'd suggest moving this out of the constructor, and adding it via a builder method.

per your suggestion make this setting a less strong requirement. I think this brings us closer to the sentiment of making this engine-level configuration without having to wire more configuration through.

lmk what you think.

A couple comments based on the discussions above:

1 (enum vs bool): If http2 were to be the only protocol we need to differentiate going forward, I think a boolean would be fine. However, since http3 is coming in the near future and we probably don't want to make protocol negotiation a hard blocker or to make breaking changes to the interfaces being introduced here, an enum seems like it'd probably make sense.

2 (engine versus request): Adding this information on a per-request basis is a bit more in line with how other configurations are set up now in Envoy Mobile. We moved to the dynamic forward proxy so that we no longer have to specify a fixed set of domains when starting the library, and requiring an http2 whitelist up front when starting up seems like a step backwards as it doesn't allow for new http2 domains to be discovered without restarting the library (not currently supported). Allowing this information to be specified per-request as a builder seems reasonable, especially considering @junr03 is stripping all x-envoy-mobile* headers (keeping the header implementation internal to the library)

@rebello95 sorry wasn't ready for a pass. I think I hit all your comments. Let me take a look