Merge pull request #1725 from stripe/richardm-move-telemetry-to-livestriperesponsegetter

Move request telemetry to LiveStripeResponseGetter

Author: richardm-stripe

build.gradle

@@ -50,6 +50,10 @@ tasks.withType(JavaCompile) {
       // com.stripe.param.AccountUpdateParams.Individual.Address] within this file.)
       // We should fix this by having autogen use the fully-qualified class to eliminate ambiguity.
       check("SameNameButDifferent", net.ltgt.gradle.errorprone.CheckSeverity.OFF)
+
+      // InlineMe (https://errorprone.info/docs/inlineme) seems neat, but in order to add these annotations
+      // we would be imposing another dependency on `errorprone` to our users, not worth it.
+      check("InlineMeSuggester", net.ltgt.gradle.errorprone.CheckSeverity.OFF)
     }
 }
 

src/main/java/com/stripe/net/HttpClient.java

@@ -3,13 +3,11 @@
 import com.stripe.Stripe;
 import com.stripe.exception.ApiConnectionException;
 import com.stripe.exception.StripeException;
-import com.stripe.util.Stopwatch;
 import java.net.ConnectException;
 import java.net.SocketTimeoutException;
 import java.time.Duration;
 import java.util.HashMap;
 import java.util.Map;
-import java.util.Optional;
 import java.util.concurrent.ThreadLocalRandom;
 
 /** Base abstract class for HTTP clients used to send requests to Stripe's API. */
@@ -20,8 +18,6 @@ public abstract class HttpClient {
   /** Minimum sleep time between tries to send HTTP requests after network failure. */
   public static final Duration minNetworkRetriesDelay = Duration.ofMillis(500);
 
-  private final RequestTelemetry requestTelemetry = new RequestTelemetry();
-
   /** A value indicating whether the client should sleep between automatic request retries. */
   boolean networkRetriesSleep = true;
 
@@ -53,47 +49,27 @@ private interface RequestSendFunction<R> {
     R apply(StripeRequest request) throws StripeException;
   }
 
-  private <T extends AbstractStripeResponse<?>> T sendWithTelemetry(
-      StripeRequest request, RequestSendFunction<T> send) throws StripeException {
-    Optional<String> telemetryHeaderValue = requestTelemetry.getHeaderValue(request.headers());
-    if (telemetryHeaderValue.isPresent()) {
-      request =
-          request.withAdditionalHeader(RequestTelemetry.HEADER_NAME, telemetryHeaderValue.get());
-    }
-
-    Stopwatch stopwatch = Stopwatch.startNew();
-
-    T response = send.apply(request);
-
-    stopwatch.stop();
-
-    requestTelemetry.maybeEnqueueMetrics(response, stopwatch.getElapsed());
-
-    return response;
-  }
-
   /**
-   * Sends the given request to Stripe's API, handling telemetry if not disabled.
-   *
    * @param request the request
    * @return the response
    * @throws StripeException If the request fails for any reason
+   * @deprecated Use {@link #request(StripeRequest)} instead.
    */
+  @Deprecated
   public StripeResponse requestWithTelemetry(StripeRequest request) throws StripeException {
-    return sendWithTelemetry(request, this::request);
+    return this.request(request);
   }
 
   /**
-   * Sends the given request to Stripe's API, streaming the response, and handling telemetry if not
-   * disabled.
-   *
    * @param request the request
    * @return the response
    * @throws StripeException If the request fails for any reason
+   * @deprecated Use {@link #requestStream(StripeRequest)} instead.
    */
+  @Deprecated
   public StripeResponseStream requestStreamWithTelemetry(StripeRequest request)
       throws StripeException {
-    return sendWithTelemetry(request, this::requestStream);
+    return this.requestStream(request);
   }
 
   public <T extends AbstractStripeResponse<?>> T sendWithRetries(
@@ -142,7 +118,7 @@ public <T extends AbstractStripeResponse<?>> T sendWithRetries(
    * @throws StripeException If the request fails for any reason
    */
   public StripeResponse requestWithRetries(StripeRequest request) throws StripeException {
-    return sendWithRetries(request, (r) -> this.requestWithTelemetry(r));
+    return sendWithRetries(request, (r) -> this.request(r));
   }
 
   /**
@@ -155,7 +131,7 @@ public StripeResponse requestWithRetries(StripeRequest request) throws StripeExc
    */
   public StripeResponseStream requestStreamWithRetries(StripeRequest request)
       throws StripeException {
-    return sendWithRetries(request, (r) -> this.requestStreamWithTelemetry(r));
+    return sendWithRetries(request, (r) -> this.requestStream(r));
   }
 
   /**

src/main/java/com/stripe/net/LiveStripeResponseGetter.java

@@ -12,15 +12,38 @@
 import com.stripe.exception.oauth.UnsupportedResponseTypeException;
 import com.stripe.model.*;
 import com.stripe.model.oauth.OAuthError;
+import com.stripe.util.Stopwatch;
 import java.io.IOException;
 import java.io.InputStream;
 import java.lang.reflect.Type;
 import java.util.Map;
+import java.util.Optional;
 
 public class LiveStripeResponseGetter implements StripeResponseGetter {
   private final HttpClient httpClient;
   private final StripeResponseGetterOptions options;
 
+  private final RequestTelemetry requestTelemetry = new RequestTelemetry();
+
+  @FunctionalInterface
+  private interface RequestSendFunction<R> {
+    R apply(StripeRequest request) throws StripeException;
+  }
+
+  private <T extends AbstractStripeResponse<?>> T sendWithTelemetry(
+      StripeRequest request, RequestSendFunction<T> send) throws StripeException {
+
+    Stopwatch stopwatch = Stopwatch.startNew();
+
+    T response = send.apply(request);
+
+    stopwatch.stop();
+
+    requestTelemetry.maybeEnqueueMetrics(response, stopwatch.getElapsed());
+
+    return response;
+  }
+
   /**
    * Initializes a new instance of the {@link LiveStripeResponseGetter} class with default
    * parameters.
@@ -46,11 +69,18 @@ public LiveStripeResponseGetter(StripeResponseGetterOptions options, HttpClient
   private StripeRequest toStripeRequest(ApiRequest apiRequest) throws StripeException {
     String fullUrl = fullUrl(apiRequest);
 
-    return new StripeRequest(
-        apiRequest.getMethod(),
-        fullUrl,
-        apiRequest.getParams(),
-        RequestOptions.merge(this.options, apiRequest.getOptions()));
+    Optional<String> telemetryHeaderValue = requestTelemetry.pollPayload();
+    StripeRequest request =
+        new StripeRequest(
+            apiRequest.getMethod(),
+            fullUrl,
+            apiRequest.getParams(),
+            RequestOptions.merge(this.options, apiRequest.getOptions()));
+    if (telemetryHeaderValue.isPresent()) {
+      request =
+          request.withAdditionalHeader(RequestTelemetry.HEADER_NAME, telemetryHeaderValue.get());
+    }
+    return request;
   }
 
   @Override
@@ -59,7 +89,7 @@ public <T extends StripeObjectInterface> T request(ApiRequest apiRequest, Type t
       throws StripeException {
 
     StripeRequest request = toStripeRequest(apiRequest);
-    StripeResponse response = httpClient.requestWithRetries(request);
+    StripeResponse response = sendWithTelemetry(request, r -> httpClient.requestWithRetries(r));
 
     int responseCode = response.code();
     String responseBody = response.body();
@@ -89,7 +119,8 @@ public <T extends StripeObjectInterface> T request(ApiRequest apiRequest, Type t
   @Override
   public InputStream requestStream(ApiRequest apiRequest) throws StripeException {
     StripeRequest request = toStripeRequest(apiRequest);
-    StripeResponseStream responseStream = httpClient.requestStreamWithRetries(request);
+    StripeResponseStream responseStream =
+        sendWithTelemetry(request, r -> httpClient.requestStreamWithRetries(r));
 
     int responseCode = responseStream.code();
 

src/main/java/com/stripe/net/RequestTelemetry.java

@@ -26,12 +26,18 @@ class RequestTelemetry {
    * metrics, or if telemetry is disabled, then the returned {@code Optional} is empty.
    *
    * @param headers the request headers
+   * @deprecated Use {$ling {@link #pollPayload()} instead.
    */
+  @Deprecated
   public Optional<String> getHeaderValue(HttpHeaders headers) {
     if (headers.firstValue(HEADER_NAME).isPresent()) {
       return Optional.empty();
     }
 
+    return this.pollPayload();
+  }
+
+  public Optional<String> pollPayload() {
     RequestMetrics requestMetrics = prevRequestMetrics.poll();
     if (requestMetrics == null) {
       return Optional.empty();