Clarify preferred ordering for [Http|Grpc]LifecycleObserver filters (…

…#2252)

Motivation:

Make javadoc of all 4 filters consistent and clarify more use-cases for
consideration when users make a decision how to append an observability
filter.

Modifications:

- Improve javadoc for `HttpLifecycleObserverServiceFilter`,
`HttpLifecycleObserverRequesterFilter`,
`GrpcLifecycleObserverServiceFilter`, and
`GrpcLifecycleObserverRequesterFilter`;

Result:

More use-cases considered in javadoc of observability filters.

servicetalk-grpc-netty/src/main/java/io/servicetalk/grpc/netty/GrpcLifecycleObserverRequesterFilter.java

@@ -15,24 +15,58 @@
  */
 package io.servicetalk.grpc.netty;
 
+import io.servicetalk.concurrent.api.AsyncContext;
 import io.servicetalk.grpc.api.GrpcClientBuilder;
 import io.servicetalk.grpc.api.GrpcLifecycleObserver;
+import io.servicetalk.grpc.api.GrpcLifecycleObserver.GrpcExchangeObserver;
+import io.servicetalk.http.api.HttpRequestMetaData;
+import io.servicetalk.http.api.HttpResponseMetaData;
 import io.servicetalk.http.api.SingleAddressHttpClientBuilder;
 import io.servicetalk.http.api.StreamingHttpClientFilterFactory;
+import io.servicetalk.http.api.StreamingHttpConnectionFilterFactory;
+import io.servicetalk.http.api.StreamingHttpResponse;
 import io.servicetalk.http.netty.HttpLifecycleObserverRequesterFilter;
 import io.servicetalk.http.netty.RetryingHttpRequesterFilter;
-import io.servicetalk.http.utils.RedirectingHttpRequesterFilter;
+import io.servicetalk.http.utils.TimeoutHttpRequesterFilter;
+
+import org.slf4j.MDC;
+
+import java.util.function.Predicate;
+import java.util.function.UnaryOperator;
 
 /**
  * An HTTP requester filter that tracks events during gRPC request/response lifecycle.
  * <p>
- * This filter is recommended to be appended as the first filter at the
- * {@link SingleAddressHttpClientBuilder#appendClientFilter(StreamingHttpClientFilterFactory) client builder}
- * (which can be configured using {@link GrpcClientBuilder#initializeHttp(GrpcClientBuilder.HttpInitializer)})
- * to account for all work done by other filters. If it's preferred to get visibility in all retried or redirected
- * requests, consider adding it after {@link RetryingHttpRequesterFilter} or {@link RedirectingHttpRequesterFilter}.
- * If it's preferred to get visibility to information populated by other filters (like tracing keys), it can be appended
- * after those filters.
+ * The result of the observed behavior will depend on the position of this filter in the execution chain.
+ * This filter is recommended to be appended as the <b>first</b>
+ * {@link SingleAddressHttpClientBuilder#appendClientFilter(StreamingHttpClientFilterFactory) client filter} using
+ * {@link GrpcClientBuilder#initializeHttp(GrpcClientBuilder.HttpInitializer)} to account for all work done by other
+ * filters. It can be appended at another position, considering the following:
+ * <ul>
+ *     <li>After a tracing filter or any other filter that populates
+ *     {@link HttpRequestMetaData#context() request context}, {@link AsyncContext}, {@link MDC}, or alters
+ *     {@link HttpRequestMetaData} if that information has to be available for {@link GrpcExchangeObserver}.</li>
+ *     <li>After {@link RetryingHttpRequesterFilter} if each retry attempt should be observed as an independent
+ *     {@link GrpcExchangeObserver exchange}.</li>
+ *     <li>As a {@link SingleAddressHttpClientBuilder#appendConnectionFilter(StreamingHttpConnectionFilterFactory)
+ *     connection filter} if no user-defined {@link RetryingHttpRequesterFilter} is appended manually but the default
+ *     auto-retries should be observed as an independent {@link GrpcExchangeObserver exchange}.</li>
+ *     <li>As the last
+ *     {@link SingleAddressHttpClientBuilder#appendConnectionFilter(StreamingHttpConnectionFilterFactory) connection
+ *     filter} if only network interactions should be observed without accounting for work of any other filters.</li>
+ *     <li>After {@link TimeoutHttpRequesterFilter} if the timeout event should be observed as
+ *     {@link GrpcExchangeObserver#onResponseCancel() cancellation} instead of an
+ *     {@link GrpcExchangeObserver#onResponseError(Throwable) error}.</li>
+ *     <li>Before any filter that populates {@link HttpResponseMetaData#context() response context} or alters
+ *     {@link HttpResponseMetaData} if that information has to be available for {@link GrpcExchangeObserver}.</li>
+ *     <li>Before any filter that maps/translates {@link HttpResponseMetaData} into an {@link Throwable exception} or
+ *     throws an exception during {@link StreamingHttpResponse#transformPayloadBody(UnaryOperator) response payload body
+ *     transformation} if that exception has to be observed by {@link GrpcExchangeObserver}.</li>
+ *     <li>Using
+ *     {@link SingleAddressHttpClientBuilder#appendClientFilter(Predicate, StreamingHttpClientFilterFactory)} or
+ *     {@link SingleAddressHttpClientBuilder#appendConnectionFilter(Predicate, StreamingHttpConnectionFilterFactory)} if
+ *     the observer should be applied conditionally.</li>
+ * </ul>
  */
 public final class GrpcLifecycleObserverRequesterFilter extends HttpLifecycleObserverRequesterFilter {
 

servicetalk-grpc-netty/src/main/java/io/servicetalk/grpc/netty/GrpcLifecycleObserverServiceFilter.java

@@ -15,25 +15,67 @@
  */
 package io.servicetalk.grpc.netty;
 
+import io.servicetalk.concurrent.api.AsyncContext;
+import io.servicetalk.grpc.api.GrpcExceptionMapperServiceFilter;
 import io.servicetalk.grpc.api.GrpcLifecycleObserver;
+import io.servicetalk.grpc.api.GrpcLifecycleObserver.GrpcExchangeObserver;
 import io.servicetalk.grpc.api.GrpcServerBuilder;
+import io.servicetalk.grpc.api.GrpcStatus;
+import io.servicetalk.http.api.HttpRequestMetaData;
+import io.servicetalk.http.api.HttpResponseMetaData;
 import io.servicetalk.http.api.HttpServerBuilder;
+import io.servicetalk.http.api.StreamingHttpResponse;
 import io.servicetalk.http.api.StreamingHttpServiceFilterFactory;
 import io.servicetalk.http.netty.HttpLifecycleObserverServiceFilter;
+import io.servicetalk.http.utils.TimeoutHttpRequesterFilter;
+import io.servicetalk.http.utils.auth.BasicAuthHttpServiceFilter;
+import io.servicetalk.transport.api.IoExecutor;
+
+import org.slf4j.MDC;
+
+import java.util.function.Predicate;
+import java.util.function.UnaryOperator;
 
 /**
  * An HTTP service filter that tracks events during gRPC request/response lifecycle.
  * <p>
- * The recommended approach is to use {@link GrpcServerBuilder#lifecycleObserver(GrpcLifecycleObserver)} to configure
- * an observer that captures entire state of the request processing. In cases when an observer should be moved down in
- * a filter chain or applied conditionally, this filter can be used.
- * <p>
- * This filter is recommended to be appended as the first filter at the
- * {@link HttpServerBuilder#appendNonOffloadingServiceFilter(StreamingHttpServiceFilterFactory)}
- * or {@link  HttpServerBuilder#appendServiceFilter(StreamingHttpServiceFilterFactory)}
- * (which can be configured using {@link GrpcServerBuilder#initializeHttp(GrpcServerBuilder.HttpInitializer)})
- * to account for all work done by other filters. If it's preferred to get visibility to information populated by other
- * filters (like tracing keys), it can be appended after those filters.
+ * The result of the observed behavior will depend on the position of this filter in the execution chain.
+ * This filter is recommended to be appended as one of the first filters using
+ * {@link HttpServerBuilder#appendNonOffloadingServiceFilter(StreamingHttpServiceFilterFactory)} method via
+ * {@link GrpcServerBuilder#initializeHttp(GrpcServerBuilder.HttpInitializer)} to account for all work done by other
+ * filters and offloading of the requests processing. It can be appended at another position, considering the following:
+ * <ul>
+ *     <li>After a tracing filter or any other filter that populates
+ *     {@link HttpRequestMetaData#context() request context}, {@link AsyncContext}, {@link MDC}, or alters
+ *     {@link HttpRequestMetaData} if that information has to be available for {@link GrpcExchangeObserver}.</li>
+ *     <li>After {@link TimeoutHttpRequesterFilter} if the timeout event should be observed as
+ *     {@link GrpcExchangeObserver#onResponseCancel() cancellation} instead of an
+ *     {@link GrpcExchangeObserver#onResponseError(Throwable) error}.</li>
+ *     <li>After any filter that can reject requests, like {@link BasicAuthHttpServiceFilter}, if only allowed requests
+ *     should be observed.</li>
+ *     <li>Before any filter that can reject requests, like {@link BasicAuthHttpServiceFilter}, if all incoming requests
+ *     have to be observed.</li>
+ *     <li>Before any filter that populates {@link HttpResponseMetaData#context() response context} or alters
+ *     {@link HttpResponseMetaData} if that information has to be available for {@link GrpcExchangeObserver}.</li>
+ *     <li>Before any filter that maps/translates {@link HttpResponseMetaData} into an {@link Throwable exception} or
+ *     throws an exception during {@link StreamingHttpResponse#transformPayloadBody(UnaryOperator) response payload body
+ *     transformation} if that exception has to be observed by {@link GrpcExchangeObserver}.</li>
+ *     <li>Before any exception mapping filter that maps an {@link Throwable exception} into a valid response, like
+ *     {@link GrpcExceptionMapperServiceFilter}, if the {@link GrpcExchangeObserver} should see what {@link GrpcStatus}
+ *     is returned to the client.</li>
+ *     <li>Using {@link HttpServerBuilder#appendServiceFilter(StreamingHttpServiceFilterFactory)} if an exchange
+ *     should be observed after it's offloaded from an {@link IoExecutor} (if offloading is enabled).</li>
+ *     <li>Using
+ *     {@link HttpServerBuilder#appendNonOffloadingServiceFilter(Predicate, StreamingHttpServiceFilterFactory)} or
+ *     {@link HttpServerBuilder#appendServiceFilter(Predicate, StreamingHttpServiceFilterFactory)} if the observer
+ *     should be applied conditionally.</li>
+ *     <li>As the last {@link HttpServerBuilder#appendServiceFilter(StreamingHttpServiceFilterFactory)} if only service
+ *     business logic should be observed without accounting for work of any other filters.</li>
+ * </ul>
+ * An alternative way to install an {@link GrpcLifecycleObserver} is to use
+ * {@link GrpcServerBuilder#lifecycleObserver(GrpcLifecycleObserver)}.
+ *
+ * @see GrpcServerBuilder#lifecycleObserver(GrpcLifecycleObserver)
  */
 public final class GrpcLifecycleObserverServiceFilter extends HttpLifecycleObserverServiceFilter {
 

servicetalk-http-netty/src/main/java/io/servicetalk/http/netty/HttpLifecycleObserverRequesterFilter.java

@@ -15,12 +15,15 @@
  */
 package io.servicetalk.http.netty;
 
+import io.servicetalk.concurrent.api.AsyncContext;
 import io.servicetalk.concurrent.api.Single;
 import io.servicetalk.http.api.FilterableReservedStreamingHttpConnection;
 import io.servicetalk.http.api.FilterableStreamingHttpClient;
 import io.servicetalk.http.api.FilterableStreamingHttpConnection;
 import io.servicetalk.http.api.HttpLifecycleObserver;
+import io.servicetalk.http.api.HttpLifecycleObserver.HttpExchangeObserver;
 import io.servicetalk.http.api.HttpRequestMetaData;
+import io.servicetalk.http.api.HttpResponseMetaData;
 import io.servicetalk.http.api.ReservedStreamingHttpConnectionFilter;
 import io.servicetalk.http.api.SingleAddressHttpClientBuilder;
 import io.servicetalk.http.api.StreamingHttpClientFilter;
@@ -31,14 +34,48 @@
 import io.servicetalk.http.api.StreamingHttpRequester;
 import io.servicetalk.http.api.StreamingHttpResponse;
 import io.servicetalk.http.utils.RedirectingHttpRequesterFilter;
+import io.servicetalk.http.utils.TimeoutHttpRequesterFilter;
+
+import org.slf4j.MDC;
+
+import java.util.function.Predicate;
+import java.util.function.UnaryOperator;
 
 /**
  * An HTTP requester filter that tracks events during request/response lifecycle.
  * <p>
- * This filter is recommended to be appended as the first filter at the
- * {@link SingleAddressHttpClientBuilder#appendClientFilter(StreamingHttpClientFilterFactory) client builder} to account
- * for all work done by other filters. If it's preferred to get visibility in all retried or redirected requests,
- * consider adding it after {@link RetryingHttpRequesterFilter} or {@link RedirectingHttpRequesterFilter}.
+ * The result of the observed behavior will depend on the position of this filter in the execution chain.
+ * This filter is recommended to be appended as the <b>first</b>
+ * {@link SingleAddressHttpClientBuilder#appendClientFilter(StreamingHttpClientFilterFactory) client filter} on the
+ * builder to account for all work done by other filters. It can be appended at another position, considering the
+ * following:
+ * <ul>
+ *     <li>After a tracing filter or any other filter that populates
+ *     {@link HttpRequestMetaData#context() request context}, {@link AsyncContext}, {@link MDC}, or alters
+ *     {@link HttpRequestMetaData} if that information has to be available for {@link HttpExchangeObserver}.</li>
+ *     <li>After {@link RedirectingHttpRequesterFilter} if each redirect should be observed as an independent
+ *     {@link HttpExchangeObserver exchange}.</li>
+ *     <li>After {@link RetryingHttpRequesterFilter} if each retry attempt should be observed as an independent
+ *     {@link HttpExchangeObserver exchange}.</li>
+ *     <li>As a {@link SingleAddressHttpClientBuilder#appendConnectionFilter(StreamingHttpConnectionFilterFactory)
+ *     connection filter} if no user-defined {@link RetryingHttpRequesterFilter} is appended manually but the default
+ *     auto-retries should be observed as an independent {@link HttpExchangeObserver exchange}.</li>
+ *     <li>As the last
+ *     {@link SingleAddressHttpClientBuilder#appendConnectionFilter(StreamingHttpConnectionFilterFactory) connection
+ *     filter} if only network interactions should be observed without accounting for work of any other filters.</li>
+ *     <li>After {@link TimeoutHttpRequesterFilter} if the timeout event should be observed as
+ *     {@link HttpExchangeObserver#onResponseCancel() cancellation} instead of an
+ *     {@link HttpExchangeObserver#onResponseError(Throwable) error}.</li>
+ *     <li>Before any filter that populates {@link HttpResponseMetaData#context() response context} or alters
+ *     {@link HttpResponseMetaData} if that information has to be available for {@link HttpExchangeObserver}.</li>
+ *     <li>Before any filter that maps/translates {@link HttpResponseMetaData} into an {@link Throwable exception} or
+ *     throws an exception during {@link StreamingHttpResponse#transformPayloadBody(UnaryOperator) response payload body
+ *     transformation} if that exception has to be observed by {@link HttpExchangeObserver}.</li>
+ *     <li>Using
+ *     {@link SingleAddressHttpClientBuilder#appendClientFilter(Predicate, StreamingHttpClientFilterFactory)} or
+ *     {@link SingleAddressHttpClientBuilder#appendConnectionFilter(Predicate, StreamingHttpConnectionFilterFactory)} if
+ *     the observer should be applied conditionally.</li>
+ * </ul>
  */
 public class HttpLifecycleObserverRequesterFilter extends AbstractLifecycleObserverHttpFilter implements
         StreamingHttpClientFilterFactory, StreamingHttpConnectionFilterFactory {

servicetalk-http-netty/src/main/java/io/servicetalk/http/netty/HttpLifecycleObserverServiceFilter.java

@@ -15,12 +15,13 @@
  */
 package io.servicetalk.http.netty;
 
+import io.servicetalk.concurrent.api.AsyncContext;
 import io.servicetalk.concurrent.api.Single;
 import io.servicetalk.http.api.HttpExceptionMapperServiceFilter;
 import io.servicetalk.http.api.HttpLifecycleObserver;
-import io.servicetalk.http.api.HttpLifecycleObserver.HttpResponseObserver;
+import io.servicetalk.http.api.HttpLifecycleObserver.HttpExchangeObserver;
+import io.servicetalk.http.api.HttpRequestMetaData;
 import io.servicetalk.http.api.HttpResponseMetaData;
-import io.servicetalk.http.api.HttpResponseStatus;
 import io.servicetalk.http.api.HttpServerBuilder;
 import io.servicetalk.http.api.HttpServiceContext;
 import io.servicetalk.http.api.StreamingHttpRequest;
@@ -29,23 +30,55 @@
 import io.servicetalk.http.api.StreamingHttpService;
 import io.servicetalk.http.api.StreamingHttpServiceFilter;
 import io.servicetalk.http.api.StreamingHttpServiceFilterFactory;
-import io.servicetalk.http.utils.TimeoutHttpServiceFilter;
+import io.servicetalk.http.utils.TimeoutHttpRequesterFilter;
+import io.servicetalk.http.utils.auth.BasicAuthHttpServiceFilter;
+import io.servicetalk.transport.api.IoExecutor;
+
+import org.slf4j.MDC;
+
+import java.util.function.Predicate;
+import java.util.function.UnaryOperator;
 
 /**
  * An HTTP service filter that tracks events during request/response lifecycle.
  * <p>
- * When this filter is used the result of the observed behavior will depend on the position of the filter in the
- * execution chain. Moving it before or after other filters, such as {@link TimeoutHttpServiceFilter}, may result in
- * different {@link HttpLifecycleObserver} callbacks being triggered
- * (seeing {@link HttpResponseObserver#onResponseCancel()} vs {@link  HttpResponseObserver#onResponseError(Throwable)}).
- * If any of the prior filters short circuit the request processing or modify {@link HttpResponseMetaData}, those won't
- * be observed. Presence of the tracing or MDC information also depends on position of this filter compare to filters
- * that populate context.
- * <p>
- * If observing the real {@link HttpResponseStatus} returned from the server is desired, consider using
- * {@link HttpServerBuilder#lifecycleObserver(HttpLifecycleObserver)} instead or place
- * {@link HttpExceptionMapperServiceFilter} right after this filter to make sure all {@link Throwable}(s) are mapped
- * into an HTTP response.
+ * The result of the observed behavior will depend on the position of this filter in the execution chain.
+ * This filter is recommended to be appended as one of the first filters using
+ * {@link HttpServerBuilder#appendNonOffloadingServiceFilter(StreamingHttpServiceFilterFactory)} method to account for
+ * all work done by other filters and offloading of the requests processing. It can be appended at another position,
+ * considering the following:
+ * <ul>
+ *     <li>After a tracing filter or any other filter that populates
+ *     {@link HttpRequestMetaData#context() request context}, {@link AsyncContext}, {@link MDC}, or alters
+ *     {@link HttpRequestMetaData} if that information has to be available for {@link HttpExchangeObserver}.</li>
+ *     <li>After {@link TimeoutHttpRequesterFilter} if the timeout event should be observed as
+ *     {@link HttpExchangeObserver#onResponseCancel() cancellation} instead of an
+ *     {@link HttpExchangeObserver#onResponseError(Throwable) error}.</li>
+ *     <li>After any filter that can reject requests, like {@link BasicAuthHttpServiceFilter}, if only allowed requests
+ *     should be observed.</li>
+ *     <li>Before any filter that can reject requests, like {@link BasicAuthHttpServiceFilter}, if all incoming requests
+ *     have to be observed.</li>
+ *     <li>Before any filter that populates {@link HttpResponseMetaData#context() response context} or alters
+ *     {@link HttpResponseMetaData} if that information has to be available for {@link HttpExchangeObserver}.</li>
+ *     <li>Before any filter that maps/translates {@link HttpResponseMetaData} into an {@link Throwable exception} or
+ *     throws an exception during {@link StreamingHttpResponse#transformPayloadBody(UnaryOperator) response payload body
+ *     transformation} if that exception has to be observed by {@link HttpExchangeObserver}.</li>
+ *     <li>Before any exception mapping filter that maps an {@link Throwable exception} into a valid response, like
+ *     {@link HttpExceptionMapperServiceFilter}, if the {@link HttpExchangeObserver} should see what status code is
+ *     returned to the client.</li>
+ *     <li>Using {@link HttpServerBuilder#appendServiceFilter(StreamingHttpServiceFilterFactory)} if an exchange
+ *     should be observed after it's offloaded from an {@link IoExecutor} (if offloading is enabled).</li>
+ *     <li>Using
+ *     {@link HttpServerBuilder#appendNonOffloadingServiceFilter(Predicate, StreamingHttpServiceFilterFactory)} or
+ *     {@link HttpServerBuilder#appendServiceFilter(Predicate, StreamingHttpServiceFilterFactory)} if the observer
+ *     should be applied conditionally.</li>
+ *     <li>As the last {@link HttpServerBuilder#appendServiceFilter(StreamingHttpServiceFilterFactory)} if only service
+ *     business logic should be observed without accounting for work of any other filters.</li>
+ * </ul>
+ * An alternative way to install an {@link HttpLifecycleObserver} is to use
+ * {@link HttpServerBuilder#lifecycleObserver(HttpLifecycleObserver)}.
+ *
+ * @see HttpServerBuilder#lifecycleObserver(HttpLifecycleObserver)
  */
 public class HttpLifecycleObserverServiceFilter extends AbstractLifecycleObserverHttpFilter implements
             StreamingHttpServiceFilterFactory {