diff --git a/src/main/java/com/orbitz/consul/monitoring/ClientEventCallback.java b/src/main/java/com/orbitz/consul/monitoring/ClientEventCallback.java index 6984b02e..b62bef74 100644 --- a/src/main/java/com/orbitz/consul/monitoring/ClientEventCallback.java +++ b/src/main/java/com/orbitz/consul/monitoring/ClientEventCallback.java @@ -6,12 +6,39 @@ public interface ClientEventCallback { + /** + * @deprecated use {@link #onHttpRequestSuccess(String, String, String, String, int)} instead. + */ + @Deprecated default void onHttpRequestSuccess(String clientName, String method, String queryString) { } + default void onHttpRequestSuccess(String clientName, String method, String path, String queryString, int responseCode) { + // Kept for compatibility at the moment + onHttpRequestSuccess(clientName, method, queryString); + } + + /** + * @deprecated use {@link #onHttpRequestFailure(String, String, String, String, Throwable)} instead. + */ + @Deprecated default void onHttpRequestFailure(String clientName, String method, String queryString, Throwable throwable) { } + default void onHttpRequestFailure(String clientName, String method, String path, String queryString, Throwable throwable) { + // Kept for compatibility at the moment + onHttpRequestFailure(clientName, method, queryString, throwable); + } + + /** + * @deprecated use {@link #onHttpRequestInvalid(String, String, String, String, int, Throwable)} instead. + */ + @Deprecated default void onHttpRequestInvalid(String clientName, String method, String queryString, Throwable throwable) { } + default void onHttpRequestInvalid(String clientName, String method, String path, String queryString, int responseCode, Throwable throwable) { + // Kept for compatibility at the moment + onHttpRequestInvalid(clientName, method, queryString, throwable); + } + default void onCacheStart(String clientName, CacheDescriptor cacheDescriptor) { } default void onCacheStop(String clientName, CacheDescriptor cacheDescriptor) { } diff --git a/src/main/java/com/orbitz/consul/monitoring/ClientEventHandler.java b/src/main/java/com/orbitz/consul/monitoring/ClientEventHandler.java index aad5a11f..a4078e58 100644 --- a/src/main/java/com/orbitz/consul/monitoring/ClientEventHandler.java +++ b/src/main/java/com/orbitz/consul/monitoring/ClientEventHandler.java @@ -2,14 +2,12 @@ import com.google.common.util.concurrent.ThreadFactoryBuilder; import com.orbitz.consul.cache.CacheDescriptor; -import okhttp3.Request; - import java.time.Duration; import java.time.temporal.ChronoUnit; -import java.time.temporal.TemporalUnit; import java.util.concurrent.Executors; import java.util.concurrent.ScheduledExecutorService; -import java.util.concurrent.TimeUnit; +import okhttp3.Request; +import retrofit2.Response; public class ClientEventHandler { @@ -24,18 +22,23 @@ public ClientEventHandler(String clientName, ClientEventCallback callback) { this.callback = callback; } - public void httpRequestSuccess(Request request) { - EVENT_EXECUTOR.submit(() -> callback.onHttpRequestSuccess(clientName, request.method(), request.url().query())); + public void httpRequestSuccess(Request request, Response response) { + EVENT_EXECUTOR.submit(() -> + callback.onHttpRequestSuccess(clientName, request.method(), request.url().encodedPath(), request.url().query(), response.code()) + ); } - public void httpRequestInvalid(Request request, Throwable throwable) { + public void httpRequestInvalid(Request request, Response response, Throwable throwable) { EVENT_EXECUTOR.submit(() -> - callback.onHttpRequestInvalid(clientName, request.method(), request.url().query(), throwable)); + callback.onHttpRequestInvalid(clientName, request.method(), request.url().encodedPath(), request.url().query(), response.code(), + throwable) + ); } public void httpRequestFailure(Request request, Throwable throwable) { EVENT_EXECUTOR.submit(() -> - callback.onHttpRequestFailure(clientName, request.method(), request.url().query(), throwable)); + callback.onHttpRequestFailure(clientName, request.method(), request.url().encodedPath(), request.url().query(), throwable) + ); } public void cacheStart(CacheDescriptor cacheDescriptor) { diff --git a/src/main/java/com/orbitz/consul/util/Http.java b/src/main/java/com/orbitz/consul/util/Http.java index 7f60a6a1..a04ea5e2 100644 --- a/src/main/java/com/orbitz/consul/util/Http.java +++ b/src/main/java/com/orbitz/consul/util/Http.java @@ -1,21 +1,19 @@ package com.orbitz.consul.util; import com.google.common.annotations.VisibleForTesting; -import com.google.common.base.Optional; import com.google.common.collect.Sets; import com.orbitz.consul.ConsulException; import com.orbitz.consul.async.Callback; import com.orbitz.consul.async.ConsulResponseCallback; import com.orbitz.consul.model.ConsulResponse; import com.orbitz.consul.monitoring.ClientEventHandler; +import java.io.IOException; +import java.math.BigInteger; import okhttp3.Headers; import org.apache.commons.lang3.math.NumberUtils; import retrofit2.Call; import retrofit2.Response; -import java.io.IOException; -import java.math.BigInteger; - public class Http { private final ClientEventHandler eventHandler; @@ -56,10 +54,10 @@ private Response executeCall(Call call) { private void ensureResponseSuccessful(Call call, Response response, Integer... okCodes) { if(isSuccessful(response, okCodes)) { - eventHandler.httpRequestSuccess(call.request()); + eventHandler.httpRequestSuccess(call.request(), response); } else { ConsulException exception = new ConsulException(response.code(), response); - eventHandler.httpRequestInvalid(call.request(), exception); + eventHandler.httpRequestInvalid(call.request(), response, exception); throw exception; } } @@ -76,11 +74,11 @@ retrofit2.Callback createCallback(Call call, final ConsulResponseCallb @Override public void onResponse(Call call, Response response) { if (isSuccessful(response, okCodes)) { - eventHandler.httpRequestSuccess(call.request()); + eventHandler.httpRequestSuccess(call.request(), response); callback.onComplete(consulResponse(response)); } else { ConsulException exception = new ConsulException(response.code(), response); - eventHandler.httpRequestInvalid(call.request(), exception); + eventHandler.httpRequestInvalid(call.request(), response, exception); callback.onFailure(exception); } } diff --git a/src/test/java/com/orbitz/consul/util/HttpTest.java b/src/test/java/com/orbitz/consul/util/HttpTest.java index 453f7fc9..c1fd842d 100644 --- a/src/test/java/com/orbitz/consul/util/HttpTest.java +++ b/src/test/java/com/orbitz/consul/util/HttpTest.java @@ -152,7 +152,7 @@ private void checkSuccessEventIsSentWhenRequestSucceed(Function, httpCall.apply(call); - verify(clientEventHandler, only()).httpRequestSuccess(any(Request.class)); + verify(clientEventHandler, only()).httpRequestSuccess(any(Request.class), any(Response.class)); } @Test @@ -209,7 +209,7 @@ private void checkInvalidEventIsSentWhenRequestIsInvalid(Function //ignore } - verify(clientEventHandler, only()).httpRequestInvalid(any(Request.class), any(Throwable.class)); + verify(clientEventHandler, only()).httpRequestInvalid(any(Request.class), any(Response.class), any(Throwable.class)); } @Test @@ -237,7 +237,7 @@ public void onFailure(Throwable throwable) { } latch.await(1, TimeUnit.SECONDS); assertEquals(expectedBody, result.get().getResponse()); - verify(clientEventHandler, only()).httpRequestSuccess(any(Request.class)); + verify(clientEventHandler, only()).httpRequestSuccess(any(Request.class), any(Response.class)); } @Test @@ -261,7 +261,7 @@ public void onFailure(Throwable throwable) { callCallback.onResponse(call, response); latch.await(1, TimeUnit.SECONDS); - verify(clientEventHandler, only()).httpRequestInvalid(any(Request.class), any(Throwable.class)); + verify(clientEventHandler, only()).httpRequestInvalid(any(Request.class), any(Response.class), any(Throwable.class)); } @Test