milamberspace commented on PR #6742:
URL: https://github.com/apache/jmeter/pull/6742#issuecomment-5266508658
Following up on removing the *HTTP Version* field — I'd argue the opposite:
keep it, but make its **options** implementation-aware instead of static.
Removing it loses something a load-testing tool specifically benefits from.
## Why keep it: comparative testing on the same target
The whole point of a field like this in JMeter is to let a single test plan
run the **same request against the same target under different explicit
protocol conditions** — e.g. HttpClient5 forced to HTTP/1.1 vs HttpClient5
negotiating HTTP/2 vs HttpClient5 strictly requiring HTTP/2, all as separate
samplers/thread groups in one scenario, to compare latency, throughput or
behavior side by side. Or Java HTTP/1.1 vs Java HTTP/2 Negotiate. That's a
legitimate, common load-testing use case (validating an HTTP/2 migration,
quantifying its actual performance benefit, or just making sure a "should be
HTTP/2" test really is one — which is exactly the failure mode my own manual
test just ran into: `NEGOTIATE` silently testing HTTP/1.1 with nothing in the
GUI making that obvious). A global `jmeter.properties` default can't do that —
it's one value for the whole run, not a per-sampler choice.
Losing per-sampler control to gain a tooltip trades away a real capability
for a cosmetic simplification.
## Proposal: implementation-aware options, not a removed field
Rather than a static 3-item combo shared by all implementations, populate it
based on the currently selected *Implementation*, dynamically (an
`ItemListener` on `httpImplementation` refreshing `httpVersion`'s model — this
also directly answers the concern about combinations that are silently ignored,
since the no-op ones simply wouldn't be offered anymore):
- **HttpClient4**: *(default)* / `HTTP/1.1` — no HTTP/2 entry at all, since
`HTTPHC4Impl` never reads `httpVersion`; today's silent no-op combination
becomes structurally impossible instead of documented-away.
- **Java**: *(default)* / `HTTP/1.1` / `HTTP/2 Negotiate` — matches what
`java.net.http.HttpClient` can actually do (no strict mode exists in the JDK's
public API, so don't offer one).
- **HttpClient5**: *(default)* / `HTTP/1.1` / `HTTP/2 Negotiate` / `HTTP/2
Strict` — HttpClient5 is the one implementation where the underlying library
can genuinely do all three, so expose all three rather than the two the current
PR limits it to.
## `HTTP/2 Strict` for HttpClient5 needs one more code change
Right now `getHttpVersionPolicy(String, String, String scheme)` only ever
returns `FORCE_HTTP_2` for plaintext `http://` with prior knowledge — for
`https://`, `HTTP/2` always resolves to `NEGOTIATE`, with silent ALPN fallback
to HTTP/1.1 if the server doesn't offer h2:
```java
static HttpVersionPolicy getHttpVersionPolicy(String samplerHttpVersion,
String defaultHttpVersion, String scheme) {
HttpVersionPolicy policy = getHttpVersionPolicy(samplerHttpVersion,
defaultHttpVersion);
if (policy == HttpVersionPolicy.NEGOTIATE && HTTP_2_PRIOR_KNOWLEDGE
&& !HTTPConstants.PROTOCOL_HTTPS.equalsIgnoreCase(scheme)) {
return HttpVersionPolicy.FORCE_HTTP_2;
}
return policy;
}
```
`ClientTlsStrategyBuilder`/`TlsConfig.setVersionPolicy(FORCE_HTTP_2)` offers
only `h2` in the ALPN extension, so the TLS handshake itself fails when the
server doesn't support it — exactly the "strict" behavior. Worth a distinct
sampler-level value (not reusing the existing `"HTTP/2"` string, to keep old
JMX files negotiating as they do today) that maps to `FORCE_HTTP_2` regardless
of scheme, surfaced as `HTTP/2 Strict` in the combo.
Happy to be told this is more scope than this PR should carry and belongs in
a follow-up — just wanted to lay out the full shape of it while the HTTP
Version field is under discussion, since the three-tier HttpClient5 combo and
the field's removal are mutually exclusive decisions.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
