Enhance visibility of connection handshake process

src/main/java/com/blazemeter/jmeter/http2/core/ConnectAttemptRecorder.java

@@ -0,0 +1,232 @@
+package com.blazemeter.jmeter.http2.core;
+
+import java.net.ConnectException;
+import java.net.InetSocketAddress;
+import java.net.SocketAddress;
+import java.net.SocketException;
+import java.net.SocketTimeoutException;
+import java.net.URL;
+import java.util.ArrayDeque;
+import java.util.ArrayList;
+import java.util.Deque;
+import java.util.List;
+import java.util.Locale;
+import java.util.Map;
+import javax.net.ssl.SSLException;
+import org.eclipse.jetty.client.Connection;
+import org.eclipse.jetty.util.Promise;
+
+/**
+ * Keeps the connection failures Jetty throws away, so a failed sample can report every address it
+ * actually tried instead of only the last one.
+ *
+ * <p>{@code HttpClient.connect(List, int, Map)} walks the resolved addresses recursively and, when
+ * one fails, calls itself for the next while <em>discarding</em> the exception - no
+ * {@code addSuppressed}, no log. Only the last address's failure survives. With the JDK default
+ * ({@code java.net.preferIPv6Addresses=false}) IPv4 is tried first, so a dual-stack host that
+ * fails on both reports the IPv6 error alone and the IPv4 one - usually the interesting one - is
+ * gone. Same code in 12.0.x and 12.1.x, so this is not a version regression.
+ *
+ * <p>The recovery point is {@code Connection.PROMISE_CONTEXT_KEY}: the promise Jetty puts in the
+ * context for one address, whose {@code failed} is exactly what decides to move on to the next.
+ * <em>Every</em> way of failing to establish a connection ends there - a refused socket, a TLS
+ * handshake, ALPN, and the HTTP/2 preface, since
+ * {@code HTTPSessionListenerPromise.failConnectionPromise} fails that same promise. Wrapping it
+ * in {@link #instrument} therefore covers all of them at once, with the address in hand, and
+ * without depending on per-phase listeners that each see only their own slice.
+ *
+ * <p>The recovered failures are attached to the sample's failure as <em>suppressed</em>
+ * exceptions: {@code HTTPSamplerBase.errorResult} writes {@code printStackTrace} into the response
+ * data, and that prints suppressed entries, so they show up in View Results Tree with no new
+ * formatting and without going through SLF4J - which the shaded jar has no provider for.
+ */
+public class ConnectAttemptRecorder {
+
+  /**
+   * Recent failures kept per client. A run that cannot connect at all would otherwise grow this
+   * without bound; the last few are all a failing sample can use, since it only reads the ones
+   * recorded while it was running.
+   */
+  private static final int DEFAULT_CAPACITY = 64;
+
+  /** Cap on how many attempts are attached to a single failure, to keep the trace readable. */
+  private static final int MAX_ATTACHED = 8;
+
+  private static final String TCP_PHASE = "Connect to";
+  private static final String TLS_PHASE = "TLS handshake with";
+  private static final String SESSION_PHASE = "Connection to";
+
+  private final int capacity;
+  private final Deque<FailedAttempt> attempts = new ArrayDeque<>();
+
+  public ConnectAttemptRecorder() {
+    this(DEFAULT_CAPACITY);
+  }
+
+  public ConnectAttemptRecorder(int capacity) {
+    this.capacity = capacity;
+  }
+
+  /**
+   * Wraps the per-address connection promise in {@code context} so this recorder sees its failure
+   * before Jetty decides to move on to the next address.
+   *
+   * <p>Called from the transport's {@code connect(SocketAddress, Map)}, which Jetty invokes once
+   * per resolved address with the promise for that address already in the context. Replacing it
+   * here is safe because everything downstream reads it back out of the context by key:
+   * {@code HttpClient.connect} binds {@code CONNECTION_PROMISE_CONTEXT_KEY} to it, and the HTTP/2
+   * session listener resolves it lazily through {@code httpConnectionPromise()}.
+   */
+  public void instrument(SocketAddress address, Map<String, Object> context) {
+    if (!(address instanceof InetSocketAddress) || context == null) {
+      return;
+    }
+    @SuppressWarnings("unchecked")
+    Promise<Connection> promise =
+        (Promise<Connection>) context.get(Connection.PROMISE_CONTEXT_KEY);
+    if (promise == null) {
+      return;
+    }
+    InetSocketAddress inetAddress = (InetSocketAddress) address;
+    context.put(Connection.PROMISE_CONTEXT_KEY, new Promise.Wrapper<Connection>(promise) {
+      @Override
+      public void failed(Throwable failure) {
+        record(inetAddress, failure);
+        super.failed(failure);
+      }
+    });
+  }
+
+  /**
+   * Attaches to {@code target} every connection failure recorded for {@code url}'s origin while
+   * the sample was running.
+   *
+   * <p>Filtering by start time is what keeps the attribution honest without tracking which sample
+   * owns which socket: connects run on Jetty threads, and concurrent embedded resources share the
+   * client, so time plus origin is the correlation available. Attempts are attached as suppressed
+   * exceptions whose cause is the original failure, keeping its stack trace intact.
+   *
+   * @param target the failure about to be reported for the sample
+   * @param url the sampled URL, used to match host and port
+   * @param sinceMillis the sample start, in epoch milliseconds; {@code 0} takes everything held
+   */
+  public void attachTo(Throwable target, URL url, long sinceMillis) {
+    if (target == null || url == null) {
+      return;
+    }
+    for (FailedAttempt attempt : failuresFor(url, sinceMillis)) {
+      // The attempt that survived Jetty's loop is already the cause of what is being reported;
+      // attaching it again would duplicate it and make printStackTrace emit a
+      // "[CIRCULAR REFERENCE]" marker. Only the discarded ones are missing.
+      if (!isInCauseChain(target, attempt.failure)) {
+        target.addSuppressed(new ConnectAttemptFailure(attempt));
+      }
+    }
+  }
+
+  private void record(InetSocketAddress address, Throwable failure) {
+    synchronized (attempts) {
+      attempts.addLast(new FailedAttempt(address, failure, System.currentTimeMillis()));
+      while (attempts.size() > capacity) {
+        attempts.removeFirst();
+      }
+    }
+  }
+
+  private static boolean isInCauseChain(Throwable target, Throwable failure) {
+    for (Throwable current = target; current != null; current = current.getCause()) {
+      if (current == failure) {
+        return true;
+      }
+      if (current.getCause() == current) {
+        break;
+      }
+    }
+    return false;
+  }
+
+  private List<FailedAttempt> failuresFor(URL url, long sinceMillis) {
+    String host = url.getHost();
+    if (host == null || host.isEmpty()) {
+      return List.of();
+    }
+    int port = url.getPort() >= 0 ? url.getPort() : url.getDefaultPort();
+    List<FailedAttempt> matches = new ArrayList<>();
+    synchronized (attempts) {
+      for (FailedAttempt attempt : attempts) {
+        if (attempt.matches(host, port, sinceMillis)) {
+          matches.add(attempt);
+        }
+      }
+    }
+    if (matches.size() > MAX_ATTACHED) {
+      return matches.subList(matches.size() - MAX_ATTACHED, matches.size());
+    }
+    return matches;
+  }
+
+  /**
+   * Names the stage the attempt died at, from the failure itself. The cause is printed underneath
+   * anyway, so this is a label rather than a diagnosis - which is what makes deriving it cheaper
+   * than wiring one listener per phase.
+   */
+  private static String phaseOf(Throwable failure) {
+    for (Throwable current = failure; current != null; current = current.getCause()) {
+      if (current instanceof SSLException) {
+        return TLS_PHASE;
+      }
+      if (current instanceof SocketException || current instanceof SocketTimeoutException
+          || current instanceof ConnectException) {
+        return TCP_PHASE;
+      }
+      if (current.getCause() == current) {
+        break;
+      }
+    }
+    return SESSION_PHASE;
+  }
+
+  private static final class FailedAttempt {
+
+    private final InetSocketAddress address;
+    private final Throwable failure;
+    private final long timestamp;
+
+    private FailedAttempt(InetSocketAddress address, Throwable failure, long timestamp) {
+      this.address = address;
+      this.failure = failure;
+      this.timestamp = timestamp;
+    }
+
+    private boolean matches(String host, int port, long sinceMillis) {
+      if (address.getPort() != port || timestamp < sinceMillis) {
+        return false;
+      }
+      // getHostString() carries the name the address was resolved from and never triggers a
+      // reverse lookup, so this stays off the network even for a literal-IP URL.
+      return address.getHostString().toLowerCase(Locale.ROOT).equals(host.toLowerCase(Locale.ROOT));
+    }
+
+    private String describe() {
+      String ip = address.getAddress() != null
+          ? address.getAddress().getHostAddress()
+          : address.getHostString();
+      return phaseOf(failure) + " " + address.getHostString() + "/" + ip + ":" + address.getPort()
+          + " failed";
+    }
+  }
+
+  /**
+   * Carrier for one discarded attempt. Its own stack trace is deliberately not filled in: the
+   * frames that matter belong to the cause, and this exception is never thrown - it exists only to
+   * be printed under {@code Suppressed:}.
+   */
+  public static class ConnectAttemptFailure extends Exception {
+
+    private static final long serialVersionUID = 1L;
+
+    private ConnectAttemptFailure(FailedAttempt attempt) {
+      super(attempt.describe(), attempt.failure, true, false);
+    }
+  }
+}

src/main/java/com/blazemeter/jmeter/http2/core/HTTP2JettyClient.java

@@ -332,6 +332,12 @@ public class HTTP2JettyClient {
    * {@code findAuthentication} (realm/URI matching quirks) and prevents per-sample list growth.
    */
   private final Set<String> registeredAuthFingerprints = ConcurrentHashMap.newKeySet();
+  /**
+   * Recovers the per-address connection failures Jetty discards while walking the resolved
+   * addresses. Shared by every connector this client builds, so an attempt is captured whichever
+   * protocol variant made it.
+   */
+  private final ConnectAttemptRecorder connectAttempts = new ConnectAttemptRecorder();
   /**
    * Every {@link ClientConnector} this client builds, so {@link #setSourceAddress} can reach the
    * QUIC one too - no transport {@code doStart} propagates the bind address to it.
@@ -600,7 +606,8 @@ public HTTP2JettyClient(boolean http1UpgradeRequired, String name,
     // even though ALPN negotiates HTTP/2 successfully. This is a regression from Jetty 11.
     // We try HTTP/2 first, then fallback to HTTP/1.1 if needed.
     ClientConnectionFactory.Info[] mainProtocols = buildMainProtocols(http3, http2, http11);
-    HttpClientTransport transport = new HttpClientTransportDynamic(clientConnector, mainProtocols);
+    HttpClientTransport transport =
+        new RecordingHttpClientTransportDynamic(clientConnector, mainProtocols);
     mainProtocolsSnapshot = protocolList(mainProtocols);
     lowLevelDebug("HttpClientTransportDynamic configured with protocols: {}",
         mainProtocolsSnapshot);
@@ -617,13 +624,14 @@ public HTTP2JettyClient(boolean http1UpgradeRequired, String name,
       ClientConnector noH3Connector = createClientConnector(name + "-noh3");
       ClientConnectionFactory.Info[] noH3Protocols = buildNoH3Protocols(http2, http11);
       HttpClientTransport noH3Transport =
-          new HttpClientTransportDynamic(noH3Connector, noH3Protocols);
+          new RecordingHttpClientTransportDynamic(noH3Connector, noH3Protocols);
       configureTransport(noH3Transport);
       this.httpClientNoH3 = new HttpClient(noH3Transport);
       configureHttpClient(this.httpClientNoH3, noH3Connector);
     }
     ClientConnector http1Connector = createClientConnector(name + "-http1");
-    HttpClientTransport http1Transport = new HttpClientTransportDynamic(http1Connector, http11);
+    HttpClientTransport http1Transport =
+        new RecordingHttpClientTransportDynamic(http1Connector, http11);
     // HTTP/1.1 has no multiplexing (Jetty rejects a 2nd in-flight exchange per connection).
     configureTransport(http1Transport, 1);
     this.httpClientHttp1Only = new HttpClient(http1Transport);
@@ -643,7 +651,7 @@ public HTTP2JettyClient(boolean http1UpgradeRequired, String name,
     ClientConnectionFactory.Info[] h2cUpgradeProtocols =
         buildH2cUpgradeProtocols(http11, http2cUpgrade);
     HttpClientTransport h2cUpgradeTransport =
-        new HttpClientTransportDynamic(h2cUpgradeConnector, h2cUpgradeProtocols);
+        new RecordingHttpClientTransportDynamic(h2cUpgradeConnector, h2cUpgradeProtocols);
     configureTransport(h2cUpgradeTransport);
     this.httpClientH2cUpgrade = new HttpClient(h2cUpgradeTransport);
     configureHttpClient(this.httpClientH2cUpgrade, h2cUpgradeConnector);
@@ -657,7 +665,8 @@ public HTTP2JettyClient(boolean http1UpgradeRequired, String name,
     } else {
       http2cClient.setMaxConcurrentPushedStreams(maxConcurrentPushedStreams);
     }
-    HttpClientTransport h2cTransport = new CustomHttpClientTransportOverHTTP2(http2cClient);
+    HttpClientTransport h2cTransport =
+        new CustomHttpClientTransportOverHTTP2(http2cClient, connectAttempts);
     configureTransport(h2cTransport);
     this.httpClientH2cPrior = new HttpClient(h2cTransport);
     configureHttpClient(this.httpClientH2cPrior, h2cConnector);
@@ -1260,7 +1269,8 @@ private HttpClient createHTTP11OnlyClient(String name) throws Exception {
 
     // Create transport with ONLY HTTP/1.1 (no HTTP/2)
     ClientConnectionFactory.Info http11 = HttpClientConnectionFactory.HTTP11;
-    HttpClientTransport transport = new HttpClientTransportDynamic(clientConnector, http11);
+    HttpClientTransport transport =
+        new RecordingHttpClientTransportDynamic(clientConnector, http11);
     lowLevelDebug("HttpClientTransportDynamic configured with HTTP/1.1 only (fallback mode)");
 
     HttpClient http11Client = new HttpClient(transport);
@@ -3362,6 +3372,28 @@ private Request cloneRequest(Request originalRequest, HttpClient client)
     return request;
   }
 
+  /**
+   * {@link HttpClientTransportDynamic} that lets {@link ConnectAttemptRecorder} see the failure of
+   * each resolved address before Jetty silently moves on to the next one.
+   *
+   * <p>This is the one point where the address being attempted and the promise that decides the
+   * roll-over are both in hand, which is why it covers a refused socket, a TLS handshake and the
+   * HTTP/2 preface alike.
+   */
+  private class RecordingHttpClientTransportDynamic extends HttpClientTransportDynamic {
+
+    RecordingHttpClientTransportDynamic(ClientConnector connector,
+                                        ClientConnectionFactory.Info... infos) {
+      super(connector, infos);
+    }
+
+    @Override
+    public void connect(SocketAddress address, Map<String, Object> context) {
+      connectAttempts.instrument(address, context);
+      super.connect(address, context);
+    }
+  }
+
   private static class HappyEyeballsThreadFactory implements ThreadFactory {
     private final String prefix;
     private final AtomicInteger counter = new AtomicInteger(1);
@@ -3510,6 +3542,14 @@ public void setSourceAddress(InetAddress sourceAddress) {
     }
   }
 
+  /**
+   * Attaches every connection failure recorded for {@code url} since {@code sinceMillis} to
+   * {@code failure} as suppressed exceptions, recovering the attempts Jetty discarded.
+   */
+  public void attachConnectAttempts(Throwable failure, URL url, long sinceMillis) {
+    connectAttempts.attachTo(failure, url, sinceMillis);
+  }
+
   private ClientConnector createClientConnector(String name) {
     ClientConnector connector = new ClientConnector();
     connectors.add(connector);
@@ -5073,4 +5113,3 @@ private void copy(InputStream input, ByteArrayOutputStream output) throws IOExce
     }
   }
 }
-

src/main/java/com/blazemeter/jmeter/http2/core/JMeterSourceAddressResolver.java

@@ -82,7 +82,7 @@ public static String cacheKeyFor(HTTPSamplerBase sampler) {
   public static boolean isConfigured(HTTPSamplerBase sampler) {
     String ipSource = sampler.getIpSource();
     return (ipSource != null && !ipSource.trim().isEmpty())
-        || !JMeterUtils.getPropDefault(LOCAL_ADDRESS_PROPERTY, "").isEmpty();
+        || !JMeterUtils.getPropDefault(LOCAL_ADDRESS_PROPERTY, "").trim().isEmpty();
   }
 
   private static InetAddress resolveIpSource(String ipSource, int ipSourceType)
@@ -131,7 +131,7 @@ private static InetAddress firstInterfaceAddress(String device,
    * is allowed to fail a sample.
    */
   private static InetAddress resolveLocalAddressProperty() {
-    String localHostOrIp = JMeterUtils.getPropDefault(LOCAL_ADDRESS_PROPERTY, "");
+    String localHostOrIp = JMeterUtils.getPropDefault(LOCAL_ADDRESS_PROPERTY, "").trim();
     if (localHostOrIp.isEmpty()) {
       return null;
     }