Addressing review.

Author: chickenchickenlove

core/src/main/java/com/linecorp/armeria/client/ClientFactoryBuilder.java

@@ -54,7 +54,6 @@
 import com.google.common.primitives.Ints;
 
 import com.linecorp.armeria.client.metric.ClientRequestLifecycleListener;
-import com.linecorp.armeria.client.metric.ClientRequestMetrics;
 import com.linecorp.armeria.client.proxy.ProxyConfig;
 import com.linecorp.armeria.client.proxy.ProxyConfigSelector;
 import com.linecorp.armeria.common.CommonPools;
@@ -263,13 +262,21 @@ public <T> ClientFactoryBuilder channelOption(ChannelOption<T> option, T value)
         channelOptions(ImmutableMap.of(option, value));
         return this;
     }
-    
-    public ClientFactoryBuilder clientRequestLifeCycleListener(ClientRequestLifecycleListener clientRequestLifecycleListener) {
-        requireNonNull(clientRequestLifecycleListener, "clientRequestLifecycleListener");
-        this.clientRequestLifecycleListener = clientRequestLifecycleListener;
+
+    /**
+     * Sets the {@link ClientRequestLifecycleListener} that listens to the lifecycle events of
+     * client requests created by the {@link ClientFactory}.
+     *
+     * <p>If not set, {@link ClientRequestLifecycleListener#noop()} is used by default.
+     *
+     * @param listener the listener to be notified of client request lifecycle events. Must not be {@code null}.
+     * @return {@code this} to support method chaining.
+     */
+    public ClientFactoryBuilder clientRequestLifecycleListener(ClientRequestLifecycleListener listener) {
+        requireNonNull(listener, "listener");
+        clientRequestLifecycleListener = listener;
         return this;
     }
-        
 
     private void channelOptions(Map<ChannelOption<?>, Object> newChannelOptions) {
         @SuppressWarnings("unchecked")
@@ -1137,8 +1144,8 @@ private ClientFactoryOptions buildOptions() {
     public ClientFactory build() {
         return new DefaultClientFactory(
                 new HttpClientFactory(
-                        buildOptions(), 
-                        autoCloseConnectionPoolListener, 
+                        buildOptions(),
+                        autoCloseConnectionPoolListener,
                         clientRequestLifecycleListener
                 )
         );

core/src/main/java/com/linecorp/armeria/client/HttpClientDelegate.java

@@ -99,6 +99,12 @@ public HttpResponse execute(ClientRequestContext ctx, HttpRequest req) throws Ex
         final DecodedHttpResponse res = new DecodedHttpResponse(eventLoop);
         updateCancellationTask(ctx, req, res);
 
+        factory.clientRequestLifecycleListener().onRequestPending(ctx);
+        ctx.log().addListener(
+                factory.clientRequestLifecycleListener()
+                        .asRequestLogListener()
+        );
+
         try {
             resolveProxyConfig(protocol, endpoint, ctx, (proxyConfig, thrown) -> {
                 if (thrown != null) {

core/src/main/java/com/linecorp/armeria/client/HttpClientFactory.java

@@ -32,14 +32,14 @@
 import java.util.function.Supplier;
 import java.util.stream.Stream;
 
-import com.linecorp.armeria.client.metric.ClientRequestLifecycleListener;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
 import com.google.common.annotations.VisibleForTesting;
 import com.google.common.collect.MapMaker;
 
 import com.linecorp.armeria.client.endpoint.EndpointGroup;
+import com.linecorp.armeria.client.metric.ClientRequestLifecycleListener;
 import com.linecorp.armeria.client.proxy.ProxyConfigSelector;
 import com.linecorp.armeria.client.redirect.RedirectConfig;
 import com.linecorp.armeria.common.Http1HeaderNaming;
@@ -145,14 +145,18 @@ private static void setupTlsMetrics(List<X509Certificate> certificates, MeterReg
     private final AsyncCloseableSupport closeable = AsyncCloseableSupport.of(this::closeAsync);
     private final ClientRequestLifecycleListener clientRequestLifecycleListener;
 
+    HttpClientFactory(ClientFactoryOptions options, boolean autoCloseConnectionPoolListener) {
+        this(options, autoCloseConnectionPoolListener, ClientRequestLifecycleListener.noop());
+    }
+
     HttpClientFactory(ClientFactoryOptions options, boolean autoCloseConnectionPoolListener,
                       ClientRequestLifecycleListener clientRequestLifecycleListener) {
         workerGroup = options.workerGroup();
 
         @SuppressWarnings("unchecked")
         final AddressResolverGroup<InetSocketAddress> group =
                 (AddressResolverGroup<InetSocketAddress>) options.addressResolverGroupFactory()
-                                                                 .apply(workerGroup);
+                        .apply(workerGroup);
         addressResolverGroup = group;
 
         final Bootstrap bootstrap = new Bootstrap();
@@ -192,10 +196,10 @@ private static void setupTlsMetrics(List<X509Certificate> certificates, MeterReg
         final TlsEngineType tlsEngineType = options.tlsEngineType();
         sslCtxHttp1Or2 = SslContextUtil
                 .createSslContext(SslContextBuilder::forClient, false, tlsEngineType,
-                                  tlsAllowUnsafeCiphers, tlsCustomizer, keyCertChainCaptor);
+                        tlsAllowUnsafeCiphers, tlsCustomizer, keyCertChainCaptor);
         sslCtxHttp1Only = SslContextUtil
                 .createSslContext(SslContextBuilder::forClient, true, tlsEngineType,
-                                  tlsAllowUnsafeCiphers, tlsCustomizer, keyCertChainCaptor);
+                        tlsAllowUnsafeCiphers, tlsCustomizer, keyCertChainCaptor);
         setupTlsMetrics(keyCertChainCaptor, options.meterRegistry());
 
         final TlsProvider tlsProvider = options.tlsProvider();
@@ -205,7 +209,7 @@ private static void setupTlsMetrics(List<X509Certificate> certificates, MeterReg
                 clientTlsConfig = null;
             }
             sslContextFactory = new SslContextFactory(tlsProvider, options.tlsEngineType(), clientTlsConfig,
-                                                      options.meterRegistry());
+                    options.meterRegistry());
         } else {
             sslContextFactory = null;
         }
@@ -342,8 +346,10 @@ ProxyConfigSelector proxyConfigSelector() {
     Http1HeaderNaming http1HeaderNaming() {
         return http1HeaderNaming;
     }
-    
-    ClientRequestLifecycleListener clientRequestLifecycleListener() {return clientRequestLifecycleListener;}
+
+    ClientRequestLifecycleListener clientRequestLifecycleListener() {
+        return clientRequestLifecycleListener;
+    }
 
     @VisibleForTesting
     AddressResolverGroup<InetSocketAddress> addressResolverGroup() {

core/src/main/java/com/linecorp/armeria/client/metric/ClientRequestLifecycleListener.java

@@ -1,15 +1,58 @@
+/*
+ * Copyright 2025 LY Corporation
+ *
+ * LY Corporation licenses this file to you under the Apache License,
+ * version 2.0 (the "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at:
+ *
+ *   https://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+ * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+ * License for the specific language governing permissions and limitations
+ * under the License.
+ */
+
 package com.linecorp.armeria.client.metric;
 
 import com.linecorp.armeria.client.ClientRequestContext;
 import com.linecorp.armeria.common.ResponseHeaders;
 import com.linecorp.armeria.common.annotation.Nullable;
 import com.linecorp.armeria.common.annotation.UnstableApi;
+import com.linecorp.armeria.common.logging.RequestLogListener;
 
+/**
+ * Listens to the lifecycle events of client requests.
+ *
+ * <p>This interface provides a high-level view of a request's lifecycle (e.g., pending, started, completed).
+ * unlike {@link RequestLogListener} which provides low-level property changes.
+ *
+ * <p>Note: The methods in this interface are typically not invoked directly by the
+ * transport layer. Instead, they are triggered by inspecting the changes in
+ * {@link com.linecorp.armeria.common.logging.RequestLog}.
+ * Implementations should use {@link #asRequestLogListener()} to bridge
+ * {@link com.linecorp.armeria.common.logging.RequestLogProperty}
+ * changes to these lifecycle methods.
+ *
+ * <p>For example, a standard implementation might map:
+ * <ul>
+ * <li>{@link com.linecorp.armeria.common.logging.RequestLogProperty#REQUEST_FIRST_BYTES_TRANSFERRED_TIME}
+ * to {@link #onRequestStart(ClientRequestContext)}</li>
+ * <li>{@link com.linecorp.armeria.common.logging.RequestLogProperty#ALL_COMPLETE}
+ * to {@link #onRequestComplete(ClientRequestContext, Throwable)}</li>
+ * </ul>
+ *
+ * @see com.linecorp.armeria.client.ClientFactoryBuilder#clientRequestLifecycleListener(
+ * ClientRequestLifecycleListener)
+ */
 @UnstableApi
 public interface ClientRequestLifecycleListener {
 
     /**
-     * Called when a request is created and scheduled but not yet executed.
+     * Invoked when a request is created and scheduled for execution but has not yet started.
+     * Note: This method is explicitly invoked by HttpClientDelegate when
+     * HttpClientDelegate starts to call execute().
      */
     void onRequestPending(ClientRequestContext ctx);
 
@@ -37,19 +80,41 @@ public interface ClientRequestLifecycleListener {
      * Called when a request is completed with either success or failure.
      */
     void onRequestComplete(ClientRequestContext ctx, @Nullable Throwable cause);
-    
+
+    /**
+     * Returns a {@link RequestLogListener} that delegates
+     * {@link com.linecorp.armeria.common.logging.RequestLog}
+     * events to this lifecycle listener.
+     * This method bridges the low-level {@link com.linecorp.armeria.common.logging.RequestLog}
+     * updates to the high-level lifecycle methods
+     * defined in this interface. The returned listener is registered to the {@link ClientRequestContext}
+     * automatically when the request starts.
+     */
+    RequestLogListener asRequestLogListener();
+
+    /**
+     * Returns a {@link ClientRequestMetrics} that collects the number of pending and active requests.
+     */
     static ClientRequestMetrics counting() {
         return new ClientRequestMetrics();
     }
-    
+
+    /**
+     * Returns a {@link ClientRequestLifecycleListener} that does nothing.
+     */
     static ClientRequestLifecycleListener noop() {
         return NoopClientRequestLifecycleListener.INSTANCE;
     }
-    
+
+    /**
+     * A {@link ClientRequestLifecycleListener} that does nothing.
+     */
     class NoopClientRequestLifecycleListener implements ClientRequestLifecycleListener {
-        
-        private static final NoopClientRequestLifecycleListener INSTANCE = new NoopClientRequestLifecycleListener();
-        
+
+        private static final NoopClientRequestLifecycleListener INSTANCE =
+                new NoopClientRequestLifecycleListener();
+        private final RequestLogListener requestLogListener = ((property, log) -> {});
+
         @Override
         public void onRequestPending(ClientRequestContext ctx) {
             // no-op
@@ -79,6 +144,11 @@ public void onResponseComplete(ClientRequestContext ctx) {
         public void onRequestComplete(ClientRequestContext ctx, @Nullable Throwable cause) {
             // no-op
         }
+
+        @Override
+        public RequestLogListener asRequestLogListener() {
+            // no-op
+            return requestLogListener;
+        }
     }
-    
 }