From a7bb9eeb5a7ef8e4bd012c29cf89391eb1de9e53 Mon Sep 17 00:00:00 2001 From: James Hageman Date: Thu, 31 Jan 2019 14:23:13 -0800 Subject: [PATCH 01/10] Add Stripe client telemetry to request headers --- src/main/java/com/stripe/Stripe.java | 1 + .../stripe/net/LiveStripeResponseGetter.java | 20 ++++++++++++++++++ .../java/com/stripe/net/RequestMetrics.java | 21 +++++++++++++++++++ 3 files changed, 42 insertions(+) create mode 100644 src/main/java/com/stripe/net/RequestMetrics.java diff --git a/src/main/java/com/stripe/Stripe.java b/src/main/java/com/stripe/Stripe.java index 7fe4f5162f8..2b42df8c5d0 100644 --- a/src/main/java/com/stripe/Stripe.java +++ b/src/main/java/com/stripe/Stripe.java @@ -19,6 +19,7 @@ public abstract class Stripe { public static volatile String apiVersion; public static volatile String clientId; public static volatile String partnerId; + public static volatile boolean enableTelemetry = false; // Note that URLConnection reserves the value of 0 to mean "infinite // timeout", so we use -1 here to represent an unset value which should diff --git a/src/main/java/com/stripe/net/LiveStripeResponseGetter.java b/src/main/java/com/stripe/net/LiveStripeResponseGetter.java index 0443d6f4e85..1b12820786b 100644 --- a/src/main/java/com/stripe/net/LiveStripeResponseGetter.java +++ b/src/main/java/com/stripe/net/LiveStripeResponseGetter.java @@ -43,6 +43,7 @@ import java.util.ListIterator; import java.util.Map; import java.util.Scanner; +import java.util.concurrent.ConcurrentLinkedQueue; import javax.net.ssl.HttpsURLConnection; import javax.net.ssl.SSLSocketFactory; @@ -50,6 +51,7 @@ public class LiveStripeResponseGetter implements StripeResponseGetter { private static final String DNS_CACHE_TTL_PROPERTY_NAME = "networkaddress.cache.ttl"; + private static final int MAX_REQUEST_METRICS_BUFFER_SIZE = 100; private static final class Parameter { public final String key; @@ -149,6 +151,12 @@ static Map getHeaders(RequestOptions options) { if (options.getStripeAccount() != null) { headers.put("Stripe-Account", options.getStripeAccount()); } + + RequestMetrics lastRequestMetrics = prevRequestMetrics.poll(); + if (Stripe.enableTelemetry && lastRequestMetrics != null) { + headers.put("X-Stripe-Client-Telemetry", ApiResource.GSON.toJson(lastRequestMetrics.payload())); + } + return headers; } @@ -385,6 +393,8 @@ private static class StripeError { String charge; } + private static ConcurrentLinkedQueue prevRequestMetrics = new ConcurrentLinkedQueue(); + // represents OAuth API errors returned as JSON // handleOAuthError uses this class to raise the appropriate OAuthException private static class StripeOAuthError { @@ -462,6 +472,7 @@ private static StripeResponse rawRequest( if (options == null) { options = RequestOptions.getDefault(); } + String originalDnsCacheTtl = null; Boolean allowedToSetTtl = true; @@ -523,8 +534,12 @@ private static T staticRequest( ApiResource.RequestMethod method, String url, Map params, Class clazz, ApiResource.RequestType type, RequestOptions options) throws StripeException { + long requestStart = System.currentTimeMillis(); + StripeResponse response = rawRequest(method, url, params, type, options); + long requestDurationMS = System.currentTimeMillis() - requestStart; + int responseCode = response.code(); String responseBody = response.body(); String requestId = response.requestId(); @@ -544,6 +559,11 @@ private static T staticRequest( StripeObject obj = (StripeObject)resource; obj.setLastResponse(response); } + + if (Stripe.enableTelemetry && prevRequestMetrics.size() < MAX_REQUEST_METRICS_BUFFER_SIZE) { + prevRequestMetrics.add(new RequestMetrics(requestId, requestDurationMS)); + } + return resource; } diff --git a/src/main/java/com/stripe/net/RequestMetrics.java b/src/main/java/com/stripe/net/RequestMetrics.java new file mode 100644 index 00000000000..c84bdfc6678 --- /dev/null +++ b/src/main/java/com/stripe/net/RequestMetrics.java @@ -0,0 +1,21 @@ +package com.stripe.net; + +public class RequestMetrics { + public String request_id; + public long request_duration_ms; + + public class Payload { + public RequestMetrics last_request_metrics; + } + + public RequestMetrics(String requestId, long requestDurationMS) { + this.request_id = requestId; + this.request_duration_ms = requestDurationMS; + } + + public Payload payload() { + Payload p = new Payload(); + p.last_request_metrics = this; + return p; + } +} From 8a641a41e03f84685d6eaa2bfd5bae239f03c132 Mon Sep 17 00:00:00 2001 From: James Hageman Date: Thu, 31 Jan 2019 15:19:30 -0800 Subject: [PATCH 02/10] Add tests with MockWebServer --- build.gradle | 1 + .../stripe/net/ClientTelemetryPayload.java | 8 ++ .../java/com/stripe/net/RequestMetrics.java | 21 +++--- .../com/stripe/functional/BalanceTest.java | 8 ++ .../com/stripe/functional/TelemetryTest.java | 75 +++++++++++++++++++ 5 files changed, 103 insertions(+), 10 deletions(-) create mode 100644 src/main/java/com/stripe/net/ClientTelemetryPayload.java create mode 100644 src/test/java/com/stripe/functional/TelemetryTest.java diff --git a/build.gradle b/build.gradle index c66ec425c5f..36a1a135795 100644 --- a/build.gradle +++ b/build.gradle @@ -47,6 +47,7 @@ dependencies { testCompile group: 'junit', name: 'junit', version:'4.12' testCompile group: 'org.mockito', name: 'mockito-core', version:'2.22.0' testRuntime group: 'org.slf4j', name: 'slf4j-api', version: '1.7.25' + testImplementation 'com.squareup.okhttp3:mockwebserver:3.12.1' } jar { diff --git a/src/main/java/com/stripe/net/ClientTelemetryPayload.java b/src/main/java/com/stripe/net/ClientTelemetryPayload.java new file mode 100644 index 00000000000..93f7941f9f8 --- /dev/null +++ b/src/main/java/com/stripe/net/ClientTelemetryPayload.java @@ -0,0 +1,8 @@ +package com.stripe.net; + +import com.google.gson.annotations.SerializedName; + +public class ClientTelemetryPayload { + @SerializedName("last_request_metrics") + public RequestMetrics lastRequestMetrics; +} diff --git a/src/main/java/com/stripe/net/RequestMetrics.java b/src/main/java/com/stripe/net/RequestMetrics.java index c84bdfc6678..0c11092d724 100644 --- a/src/main/java/com/stripe/net/RequestMetrics.java +++ b/src/main/java/com/stripe/net/RequestMetrics.java @@ -1,21 +1,22 @@ package com.stripe.net; +import com.google.gson.annotations.SerializedName; + public class RequestMetrics { - public String request_id; - public long request_duration_ms; + @SerializedName("request_id") + public String requestId; - public class Payload { - public RequestMetrics last_request_metrics; - } + @SerializedName("request_duration_ms") + public long requestDurationMs; public RequestMetrics(String requestId, long requestDurationMS) { - this.request_id = requestId; - this.request_duration_ms = requestDurationMS; + this.requestId = requestId; + this.requestDurationMs = requestDurationMS; } - public Payload payload() { - Payload p = new Payload(); - p.last_request_metrics = this; + public ClientTelemetryPayload payload() { + ClientTelemetryPayload p = new ClientTelemetryPayload(); + p.lastRequestMetrics = this; return p; } } diff --git a/src/test/java/com/stripe/functional/BalanceTest.java b/src/test/java/com/stripe/functional/BalanceTest.java index 3fcb6ef1bee..49b69e08ac4 100644 --- a/src/test/java/com/stripe/functional/BalanceTest.java +++ b/src/test/java/com/stripe/functional/BalanceTest.java @@ -1,14 +1,22 @@ package com.stripe.functional; import static org.junit.Assert.assertNotNull; +import static org.junit.Assert.assertNull; + +import com.stripe.Stripe; import com.stripe.BaseStripeTest; import com.stripe.exception.StripeException; import com.stripe.model.Balance; import com.stripe.net.ApiResource; +import okhttp3.mockwebserver.MockResponse; +import okhttp3.mockwebserver.MockWebServer; +import okhttp3.mockwebserver.RecordedRequest; import org.junit.Test; +import java.io.IOException; + public class BalanceTest extends BaseStripeTest { @Test public void testRetrieve() throws StripeException { diff --git a/src/test/java/com/stripe/functional/TelemetryTest.java b/src/test/java/com/stripe/functional/TelemetryTest.java new file mode 100644 index 00000000000..5011ca69cd6 --- /dev/null +++ b/src/test/java/com/stripe/functional/TelemetryTest.java @@ -0,0 +1,75 @@ +package com.stripe.functional; + +import com.stripe.BaseStripeTest; +import com.stripe.Stripe; +import com.stripe.exception.StripeException; +import com.stripe.model.Balance; +import com.stripe.net.ApiResource; +import com.stripe.net.ClientTelemetryPayload; +import okhttp3.mockwebserver.MockResponse; +import okhttp3.mockwebserver.MockWebServer; +import okhttp3.mockwebserver.RecordedRequest; +import org.junit.Test; + +import java.io.IOException; +import java.util.concurrent.TimeUnit; + +import static org.junit.Assert.*; + +public class TelemetryTest extends BaseStripeTest { + @Test + public void testTelemetryEnabled() throws StripeException, IOException, InterruptedException { + MockWebServer server = new MockWebServer(); + server.enqueue(new MockResponse().setBody("{}").addHeader("Request-Id", "req_1").setBodyDelay(30, TimeUnit.MILLISECONDS)); + server.enqueue(new MockResponse().setBody("{}").addHeader("Request-Id", "req_2").setBodyDelay(70, TimeUnit.MILLISECONDS)); + server.enqueue(new MockResponse().setBody("{}").addHeader("Request-Id", "req_3")); + server.start(); + + Stripe.overrideApiBase(server.url("").toString()); + Stripe.enableTelemetry = true; + + Balance b1 = Balance.retrieve(); + RecordedRequest request1 = server.takeRequest(); + assertNull(request1.getHeader("X-Stripe-Client-Telemetry")); + + Balance b2 = Balance.retrieve(); + RecordedRequest request2 = server.takeRequest(); + String telemetry1 = request2.getHeader("X-Stripe-Client-Telemetry"); + ClientTelemetryPayload payload1 = ApiResource.GSON.fromJson(telemetry1, ClientTelemetryPayload.class); + assertEquals(payload1.lastRequestMetrics.requestId, "req_1"); + assertTrue(payload1.lastRequestMetrics.requestDurationMs > 30); + assertTrue(payload1.lastRequestMetrics.requestDurationMs < 60); + + Balance b3 = Balance.retrieve(); + RecordedRequest request3 = server.takeRequest(); + String telemetry2 = request3.getHeader("X-Stripe-Client-Telemetry"); + ClientTelemetryPayload payload2 = ApiResource.GSON.fromJson(telemetry2, ClientTelemetryPayload.class); + assertEquals(payload2.lastRequestMetrics.requestId, "req_2"); + assertTrue(payload2.lastRequestMetrics.requestDurationMs > 70); + assertTrue(payload2.lastRequestMetrics.requestDurationMs < 100); + + server.shutdown(); + } + + @Test + public void testTelemetryDisabled() throws StripeException, IOException, InterruptedException { + MockWebServer server = new MockWebServer(); + server.enqueue(new MockResponse().setBody("{}").addHeader("Request-Id", "req_1")); + server.enqueue(new MockResponse().setBody("{}").addHeader("Request-Id", "req_2")); + server.enqueue(new MockResponse().setBody("{}").addHeader("Request-Id", "req_3")); + server.start(); + + Stripe.overrideApiBase(server.url("").toString()); + Stripe.enableTelemetry = false; + + Balance b1 = Balance.retrieve(); + RecordedRequest request1 = server.takeRequest(); + assertNull(request1.getHeader("X-Stripe-Client-Telemetry")); + + Balance b2 = Balance.retrieve(); + RecordedRequest request2 = server.takeRequest(); + assertNull(request2.getHeader("X-Stripe-Client-Telemetry")); + + server.shutdown(); + } +} From 745323cd215fb292caea516c412b7dbd86624c00 Mon Sep 17 00:00:00 2001 From: James Hageman Date: Thu, 31 Jan 2019 15:33:36 -0800 Subject: [PATCH 03/10] Add a test for thread safety --- .../com/stripe/functional/TelemetryTest.java | 71 +++++++++++++++++++ 1 file changed, 71 insertions(+) diff --git a/src/test/java/com/stripe/functional/TelemetryTest.java b/src/test/java/com/stripe/functional/TelemetryTest.java index 5011ca69cd6..bc057b0dd43 100644 --- a/src/test/java/com/stripe/functional/TelemetryTest.java +++ b/src/test/java/com/stripe/functional/TelemetryTest.java @@ -12,6 +12,9 @@ import org.junit.Test; import java.io.IOException; +import java.util.ArrayList; +import java.util.HashSet; +import java.util.Set; import java.util.concurrent.TimeUnit; import static org.junit.Assert.*; @@ -72,4 +75,72 @@ public void testTelemetryDisabled() throws StripeException, IOException, Interru server.shutdown(); } + + @Test + public void testTelemetryWorksWithConcurrentRequests() throws IOException, InterruptedException { + MockWebServer server = new MockWebServer(); + + for (int i = 0; i < 20; i++) { + server.enqueue(new MockResponse().setBody("{}").addHeader("Request-Id", "req_" + i)); + } + server.start(); + + Stripe.overrideApiBase(server.url("").toString()); + Stripe.enableTelemetry = true; + + Runnable work = new Runnable() { + @Override + public void run() { + try { + Balance.retrieve(); + } catch (StripeException e) { + assertNull(e); + } + } + }; + + // the first 10 requests will not contain telemetry + ArrayList threads = new ArrayList<>(); + for (int i = 0; i < 10; i++) { + threads.add(new Thread(work)); + } + for (int i = 0; i < 10; i++) { + threads.get(i).start(); + } + for (int i = 0; i < 10; i++) { + threads.get(i).join(); + } + threads.clear(); + + // the following 10 requests will contain telemetry + for (int i = 0; i < 10; i++) { + threads.add(new Thread(work)); + } + for (int i = 0; i < 10; i++) { + threads.get(i).start(); + } + for (int i = 0; i < 10; i++) { + threads.get(i).join(); + } + threads.clear(); + + Set seenRequestIds = new HashSet<>(); + + for (int i = 0; i < 10; i++) { + RecordedRequest request = server.takeRequest(); + assertNull(request.getHeader("X-Stripe-Client-Telemetry")); + } + + for (int i = 0; i < 10; i++) { + RecordedRequest request = server.takeRequest(); + String telemetry2 = request.getHeader("X-Stripe-Client-Telemetry"); + ClientTelemetryPayload payload = ApiResource.GSON.fromJson(telemetry2, ClientTelemetryPayload.class); + seenRequestIds.add(payload.lastRequestMetrics.requestId); + } + + // check that each telemetry payload corresponds to a unique request id + assertEquals(10, seenRequestIds.size()); + + server.shutdown(); + } } From 8926b5b732d4f68bb98cd4a0b3602069f7eeefa7 Mon Sep 17 00:00:00 2001 From: James Hageman Date: Thu, 31 Jan 2019 15:50:24 -0800 Subject: [PATCH 04/10] Reformatting --- .../stripe/net/ClientTelemetryPayload.java | 4 +-- .../stripe/net/LiveStripeResponseGetter.java | 6 ++-- .../java/com/stripe/net/RequestMetrics.java | 29 +++++++++------- .../com/stripe/functional/BalanceTest.java | 8 ----- .../com/stripe/functional/TelemetryTest.java | 34 ++++++++++++------- 5 files changed, 44 insertions(+), 37 deletions(-) diff --git a/src/main/java/com/stripe/net/ClientTelemetryPayload.java b/src/main/java/com/stripe/net/ClientTelemetryPayload.java index 93f7941f9f8..9b7e0ef2bef 100644 --- a/src/main/java/com/stripe/net/ClientTelemetryPayload.java +++ b/src/main/java/com/stripe/net/ClientTelemetryPayload.java @@ -3,6 +3,6 @@ import com.google.gson.annotations.SerializedName; public class ClientTelemetryPayload { - @SerializedName("last_request_metrics") - public RequestMetrics lastRequestMetrics; + @SerializedName("last_request_metrics") + public RequestMetrics lastRequestMetrics; } diff --git a/src/main/java/com/stripe/net/LiveStripeResponseGetter.java b/src/main/java/com/stripe/net/LiveStripeResponseGetter.java index 1b12820786b..9dd06e27810 100644 --- a/src/main/java/com/stripe/net/LiveStripeResponseGetter.java +++ b/src/main/java/com/stripe/net/LiveStripeResponseGetter.java @@ -154,7 +154,8 @@ static Map getHeaders(RequestOptions options) { RequestMetrics lastRequestMetrics = prevRequestMetrics.poll(); if (Stripe.enableTelemetry && lastRequestMetrics != null) { - headers.put("X-Stripe-Client-Telemetry", ApiResource.GSON.toJson(lastRequestMetrics.payload())); + headers.put("X-Stripe-Client-Telemetry", + ApiResource.GSON.toJson(lastRequestMetrics.payload())); } return headers; @@ -393,7 +394,8 @@ private static class StripeError { String charge; } - private static ConcurrentLinkedQueue prevRequestMetrics = new ConcurrentLinkedQueue(); + private static ConcurrentLinkedQueue prevRequestMetrics = + new ConcurrentLinkedQueue(); // represents OAuth API errors returned as JSON // handleOAuthError uses this class to raise the appropriate OAuthException diff --git a/src/main/java/com/stripe/net/RequestMetrics.java b/src/main/java/com/stripe/net/RequestMetrics.java index 0c11092d724..021f3f3750c 100644 --- a/src/main/java/com/stripe/net/RequestMetrics.java +++ b/src/main/java/com/stripe/net/RequestMetrics.java @@ -3,20 +3,23 @@ import com.google.gson.annotations.SerializedName; public class RequestMetrics { - @SerializedName("request_id") - public String requestId; + @SerializedName("request_id") + public String requestId; - @SerializedName("request_duration_ms") - public long requestDurationMs; + @SerializedName("request_duration_ms") + public long requestDurationMs; - public RequestMetrics(String requestId, long requestDurationMS) { - this.requestId = requestId; - this.requestDurationMs = requestDurationMS; - } + public RequestMetrics(String requestId, long requestDurationMS) { + this.requestId = requestId; + this.requestDurationMs = requestDurationMS; + } - public ClientTelemetryPayload payload() { - ClientTelemetryPayload p = new ClientTelemetryPayload(); - p.lastRequestMetrics = this; - return p; - } + /** + * Constructs the JSON payload to be sent in the X-Stripe-Client-Telemetry header. + */ + public ClientTelemetryPayload payload() { + ClientTelemetryPayload p = new ClientTelemetryPayload(); + p.lastRequestMetrics = this; + return p; + } } diff --git a/src/test/java/com/stripe/functional/BalanceTest.java b/src/test/java/com/stripe/functional/BalanceTest.java index 49b69e08ac4..3fcb6ef1bee 100644 --- a/src/test/java/com/stripe/functional/BalanceTest.java +++ b/src/test/java/com/stripe/functional/BalanceTest.java @@ -1,22 +1,14 @@ package com.stripe.functional; import static org.junit.Assert.assertNotNull; -import static org.junit.Assert.assertNull; - -import com.stripe.Stripe; import com.stripe.BaseStripeTest; import com.stripe.exception.StripeException; import com.stripe.model.Balance; import com.stripe.net.ApiResource; -import okhttp3.mockwebserver.MockResponse; -import okhttp3.mockwebserver.MockWebServer; -import okhttp3.mockwebserver.RecordedRequest; import org.junit.Test; -import java.io.IOException; - public class BalanceTest extends BaseStripeTest { @Test public void testRetrieve() throws StripeException { diff --git a/src/test/java/com/stripe/functional/TelemetryTest.java b/src/test/java/com/stripe/functional/TelemetryTest.java index bc057b0dd43..099f023e8ea 100644 --- a/src/test/java/com/stripe/functional/TelemetryTest.java +++ b/src/test/java/com/stripe/functional/TelemetryTest.java @@ -1,15 +1,15 @@ package com.stripe.functional; +import static junit.framework.TestCase.assertEquals; +import static junit.framework.TestCase.assertNull; +import static junit.framework.TestCase.assertTrue; + import com.stripe.BaseStripeTest; import com.stripe.Stripe; import com.stripe.exception.StripeException; import com.stripe.model.Balance; import com.stripe.net.ApiResource; import com.stripe.net.ClientTelemetryPayload; -import okhttp3.mockwebserver.MockResponse; -import okhttp3.mockwebserver.MockWebServer; -import okhttp3.mockwebserver.RecordedRequest; -import org.junit.Test; import java.io.IOException; import java.util.ArrayList; @@ -17,14 +17,20 @@ import java.util.Set; import java.util.concurrent.TimeUnit; -import static org.junit.Assert.*; +import okhttp3.mockwebserver.MockResponse; +import okhttp3.mockwebserver.MockWebServer; +import okhttp3.mockwebserver.RecordedRequest; +import org.junit.Test; + public class TelemetryTest extends BaseStripeTest { @Test public void testTelemetryEnabled() throws StripeException, IOException, InterruptedException { MockWebServer server = new MockWebServer(); - server.enqueue(new MockResponse().setBody("{}").addHeader("Request-Id", "req_1").setBodyDelay(30, TimeUnit.MILLISECONDS)); - server.enqueue(new MockResponse().setBody("{}").addHeader("Request-Id", "req_2").setBodyDelay(70, TimeUnit.MILLISECONDS)); + server.enqueue(new MockResponse().setBody("{}").addHeader("Request-Id", "req_1") + .setBodyDelay(30, TimeUnit.MILLISECONDS)); + server.enqueue(new MockResponse().setBody("{}").addHeader("Request-Id", "req_2") + .setBodyDelay(70, TimeUnit.MILLISECONDS)); server.enqueue(new MockResponse().setBody("{}").addHeader("Request-Id", "req_3")); server.start(); @@ -38,7 +44,8 @@ public void testTelemetryEnabled() throws StripeException, IOException, Interrup Balance b2 = Balance.retrieve(); RecordedRequest request2 = server.takeRequest(); String telemetry1 = request2.getHeader("X-Stripe-Client-Telemetry"); - ClientTelemetryPayload payload1 = ApiResource.GSON.fromJson(telemetry1, ClientTelemetryPayload.class); + ClientTelemetryPayload payload1 = ApiResource.GSON.fromJson( + telemetry1, ClientTelemetryPayload.class); assertEquals(payload1.lastRequestMetrics.requestId, "req_1"); assertTrue(payload1.lastRequestMetrics.requestDurationMs > 30); assertTrue(payload1.lastRequestMetrics.requestDurationMs < 60); @@ -46,7 +53,8 @@ public void testTelemetryEnabled() throws StripeException, IOException, Interrup Balance b3 = Balance.retrieve(); RecordedRequest request3 = server.takeRequest(); String telemetry2 = request3.getHeader("X-Stripe-Client-Telemetry"); - ClientTelemetryPayload payload2 = ApiResource.GSON.fromJson(telemetry2, ClientTelemetryPayload.class); + ClientTelemetryPayload payload2 = ApiResource.GSON.fromJson( + telemetry2, ClientTelemetryPayload.class); assertEquals(payload2.lastRequestMetrics.requestId, "req_2"); assertTrue(payload2.lastRequestMetrics.requestDurationMs > 70); assertTrue(payload2.lastRequestMetrics.requestDurationMs < 100); @@ -81,7 +89,8 @@ public void testTelemetryWorksWithConcurrentRequests() throws IOException, Inter MockWebServer server = new MockWebServer(); for (int i = 0; i < 20; i++) { - server.enqueue(new MockResponse().setBody("{}").addHeader("Request-Id", "req_" + i)); + server.enqueue(new MockResponse().setBody("{}") + .addHeader("Request-Id", "req_" + i)); } server.start(); @@ -102,7 +111,7 @@ public void run() { // the first 10 requests will not contain telemetry ArrayList threads = new ArrayList<>(); for (int i = 0; i < 10; i++) { - threads.add(new Thread(work)); + threads.add(new Thread(work)); } for (int i = 0; i < 10; i++) { threads.get(i).start(); @@ -134,7 +143,8 @@ public void run() { for (int i = 0; i < 10; i++) { RecordedRequest request = server.takeRequest(); String telemetry2 = request.getHeader("X-Stripe-Client-Telemetry"); - ClientTelemetryPayload payload = ApiResource.GSON.fromJson(telemetry2, ClientTelemetryPayload.class); + ClientTelemetryPayload payload = ApiResource.GSON.fromJson( + telemetry2, ClientTelemetryPayload.class); seenRequestIds.add(payload.lastRequestMetrics.requestId); } From 8878fd786f1547352ff215ac5ca4f134f5ec851d Mon Sep 17 00:00:00 2001 From: James Hageman Date: Thu, 31 Jan 2019 15:55:55 -0800 Subject: [PATCH 05/10] add delay so all spawned threads can start --- src/test/java/com/stripe/functional/TelemetryTest.java | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/test/java/com/stripe/functional/TelemetryTest.java b/src/test/java/com/stripe/functional/TelemetryTest.java index 099f023e8ea..46a8051738d 100644 --- a/src/test/java/com/stripe/functional/TelemetryTest.java +++ b/src/test/java/com/stripe/functional/TelemetryTest.java @@ -90,7 +90,8 @@ public void testTelemetryWorksWithConcurrentRequests() throws IOException, Inter for (int i = 0; i < 20; i++) { server.enqueue(new MockResponse().setBody("{}") - .addHeader("Request-Id", "req_" + i)); + .addHeader("Request-Id", "req_" + i) + .setBodyDelay(50, TimeUnit.MILLISECONDS)); } server.start(); From 466198d1baa4b8bce8467791a994b9ba34d2a0fd Mon Sep 17 00:00:00 2001 From: James Hageman Date: Thu, 31 Jan 2019 16:01:35 -0800 Subject: [PATCH 06/10] Loosen timing assertions --- src/test/java/com/stripe/functional/TelemetryTest.java | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/test/java/com/stripe/functional/TelemetryTest.java b/src/test/java/com/stripe/functional/TelemetryTest.java index 46a8051738d..c98cc399412 100644 --- a/src/test/java/com/stripe/functional/TelemetryTest.java +++ b/src/test/java/com/stripe/functional/TelemetryTest.java @@ -30,7 +30,7 @@ public void testTelemetryEnabled() throws StripeException, IOException, Interrup server.enqueue(new MockResponse().setBody("{}").addHeader("Request-Id", "req_1") .setBodyDelay(30, TimeUnit.MILLISECONDS)); server.enqueue(new MockResponse().setBody("{}").addHeader("Request-Id", "req_2") - .setBodyDelay(70, TimeUnit.MILLISECONDS)); + .setBodyDelay(120, TimeUnit.MILLISECONDS)); server.enqueue(new MockResponse().setBody("{}").addHeader("Request-Id", "req_3")); server.start(); @@ -48,7 +48,7 @@ public void testTelemetryEnabled() throws StripeException, IOException, Interrup telemetry1, ClientTelemetryPayload.class); assertEquals(payload1.lastRequestMetrics.requestId, "req_1"); assertTrue(payload1.lastRequestMetrics.requestDurationMs > 30); - assertTrue(payload1.lastRequestMetrics.requestDurationMs < 60); + assertTrue(payload1.lastRequestMetrics.requestDurationMs < 130); Balance b3 = Balance.retrieve(); RecordedRequest request3 = server.takeRequest(); @@ -56,8 +56,8 @@ public void testTelemetryEnabled() throws StripeException, IOException, Interrup ClientTelemetryPayload payload2 = ApiResource.GSON.fromJson( telemetry2, ClientTelemetryPayload.class); assertEquals(payload2.lastRequestMetrics.requestId, "req_2"); - assertTrue(payload2.lastRequestMetrics.requestDurationMs > 70); - assertTrue(payload2.lastRequestMetrics.requestDurationMs < 100); + assertTrue(payload2.lastRequestMetrics.requestDurationMs > 120); + assertTrue(payload2.lastRequestMetrics.requestDurationMs < 220); server.shutdown(); } From d00ed48db56e7d5314025348074c6af2085874db Mon Sep 17 00:00:00 2001 From: James Hageman Date: Thu, 31 Jan 2019 16:06:38 -0800 Subject: [PATCH 07/10] Remove upper bounds in timing assertions because slowness is inevitable --- src/test/java/com/stripe/functional/TelemetryTest.java | 2 -- 1 file changed, 2 deletions(-) diff --git a/src/test/java/com/stripe/functional/TelemetryTest.java b/src/test/java/com/stripe/functional/TelemetryTest.java index c98cc399412..fa2c34a2cc2 100644 --- a/src/test/java/com/stripe/functional/TelemetryTest.java +++ b/src/test/java/com/stripe/functional/TelemetryTest.java @@ -48,7 +48,6 @@ public void testTelemetryEnabled() throws StripeException, IOException, Interrup telemetry1, ClientTelemetryPayload.class); assertEquals(payload1.lastRequestMetrics.requestId, "req_1"); assertTrue(payload1.lastRequestMetrics.requestDurationMs > 30); - assertTrue(payload1.lastRequestMetrics.requestDurationMs < 130); Balance b3 = Balance.retrieve(); RecordedRequest request3 = server.takeRequest(); @@ -57,7 +56,6 @@ public void testTelemetryEnabled() throws StripeException, IOException, Interrup telemetry2, ClientTelemetryPayload.class); assertEquals(payload2.lastRequestMetrics.requestId, "req_2"); assertTrue(payload2.lastRequestMetrics.requestDurationMs > 120); - assertTrue(payload2.lastRequestMetrics.requestDurationMs < 220); server.shutdown(); } From a956afc3f6bfc2b8e9dcd2f3a04186b06a07d82c Mon Sep 17 00:00:00 2001 From: James Hageman Date: Thu, 31 Jan 2019 16:13:28 -0800 Subject: [PATCH 08/10] Remove extraneous newline --- src/main/java/com/stripe/net/LiveStripeResponseGetter.java | 1 - 1 file changed, 1 deletion(-) diff --git a/src/main/java/com/stripe/net/LiveStripeResponseGetter.java b/src/main/java/com/stripe/net/LiveStripeResponseGetter.java index 9dd06e27810..0b428e2e337 100644 --- a/src/main/java/com/stripe/net/LiveStripeResponseGetter.java +++ b/src/main/java/com/stripe/net/LiveStripeResponseGetter.java @@ -474,7 +474,6 @@ private static StripeResponse rawRequest( if (options == null) { options = RequestOptions.getDefault(); } - String originalDnsCacheTtl = null; Boolean allowedToSetTtl = true; From 18ee5c60585b58da30be5f3d6b92474f21d1396e Mon Sep 17 00:00:00 2001 From: James Hageman Date: Thu, 31 Jan 2019 17:30:21 -0800 Subject: [PATCH 09/10] clean ups --- .../com/stripe/functional/TelemetryTest.java | 19 ++++++++----------- 1 file changed, 8 insertions(+), 11 deletions(-) diff --git a/src/test/java/com/stripe/functional/TelemetryTest.java b/src/test/java/com/stripe/functional/TelemetryTest.java index fa2c34a2cc2..ce8fab2889b 100644 --- a/src/test/java/com/stripe/functional/TelemetryTest.java +++ b/src/test/java/com/stripe/functional/TelemetryTest.java @@ -110,10 +110,9 @@ public void run() { // the first 10 requests will not contain telemetry ArrayList threads = new ArrayList<>(); for (int i = 0; i < 10; i++) { - threads.add(new Thread(work)); - } - for (int i = 0; i < 10; i++) { - threads.get(i).start(); + Thread t = new Thread(work); + threads.add(t); + t.start(); } for (int i = 0; i < 10; i++) { threads.get(i).join(); @@ -122,15 +121,13 @@ public void run() { // the following 10 requests will contain telemetry for (int i = 0; i < 10; i++) { - threads.add(new Thread(work)); - } - for (int i = 0; i < 10; i++) { - threads.get(i).start(); + Thread t = new Thread(work); + threads.add(t); + t.start(); } for (int i = 0; i < 10; i++) { threads.get(i).join(); } - threads.clear(); Set seenRequestIds = new HashSet<>(); @@ -141,9 +138,9 @@ public void run() { for (int i = 0; i < 10; i++) { RecordedRequest request = server.takeRequest(); - String telemetry2 = request.getHeader("X-Stripe-Client-Telemetry"); + String telemetry = request.getHeader("X-Stripe-Client-Telemetry"); ClientTelemetryPayload payload = ApiResource.GSON.fromJson( - telemetry2, ClientTelemetryPayload.class); + telemetry, ClientTelemetryPayload.class); seenRequestIds.add(payload.lastRequestMetrics.requestId); } From fced9b8110e58618d4a808de86a7980a3254720d Mon Sep 17 00:00:00 2001 From: James Hageman Date: Mon, 4 Feb 2019 10:54:37 -0800 Subject: [PATCH 10/10] address minor comments --- src/main/java/com/stripe/Stripe.java | 2 +- src/main/java/com/stripe/net/LiveStripeResponseGetter.java | 6 +++--- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/src/main/java/com/stripe/Stripe.java b/src/main/java/com/stripe/Stripe.java index 2b42df8c5d0..89102f83832 100644 --- a/src/main/java/com/stripe/Stripe.java +++ b/src/main/java/com/stripe/Stripe.java @@ -18,8 +18,8 @@ public abstract class Stripe { public static volatile String apiKey; public static volatile String apiVersion; public static volatile String clientId; - public static volatile String partnerId; public static volatile boolean enableTelemetry = false; + public static volatile String partnerId; // Note that URLConnection reserves the value of 0 to mean "infinite // timeout", so we use -1 here to represent an unset value which should diff --git a/src/main/java/com/stripe/net/LiveStripeResponseGetter.java b/src/main/java/com/stripe/net/LiveStripeResponseGetter.java index 0b428e2e337..ff95fcab9ae 100644 --- a/src/main/java/com/stripe/net/LiveStripeResponseGetter.java +++ b/src/main/java/com/stripe/net/LiveStripeResponseGetter.java @@ -535,11 +535,11 @@ private static T staticRequest( ApiResource.RequestMethod method, String url, Map params, Class clazz, ApiResource.RequestType type, RequestOptions options) throws StripeException { - long requestStart = System.currentTimeMillis(); + long requestStartMs = System.currentTimeMillis(); StripeResponse response = rawRequest(method, url, params, type, options); - long requestDurationMS = System.currentTimeMillis() - requestStart; + long requestDurationMs = System.currentTimeMillis() - requestStartMs; int responseCode = response.code(); String responseBody = response.body(); @@ -562,7 +562,7 @@ private static T staticRequest( } if (Stripe.enableTelemetry && prevRequestMetrics.size() < MAX_REQUEST_METRICS_BUFFER_SIZE) { - prevRequestMetrics.add(new RequestMetrics(requestId, requestDurationMS)); + prevRequestMetrics.add(new RequestMetrics(requestId, requestDurationMs)); } return resource;