make http client configurable, closes #30 (#31)

src/main/java/ratpack/zipkin/ServerTracingModule.java

@@ -28,13 +28,16 @@
 import com.google.inject.Provides;
 import com.google.inject.Singleton;
 import com.google.inject.multibindings.Multibinder;
+import com.google.inject.multibindings.OptionalBinder;
+import ratpack.func.Action;
 import ratpack.guice.ConfigurableModule;
 import ratpack.handling.HandlerDecorator;
 import ratpack.http.client.HttpClient;
 import ratpack.server.ServerConfig;
 import ratpack.util.Exceptions;
 import ratpack.zipkin.internal.DefaultClientTracingInterceptor;
 import ratpack.zipkin.internal.DefaultServerTracingHandler;
+import ratpack.zipkin.internal.HttpClientProvider;
 import ratpack.zipkin.internal.RatpackCurrentTraceContext;
 import ratpack.zipkin.internal.RatpackHttpServerParser;
 import zipkin2.Span;
@@ -55,22 +58,12 @@ protected void configure() {
         .to(DefaultClientTracingInterceptor.class)
         .in(Singleton.class);
 
-    Provider<ClientTracingInterceptor> clientTracingInterceptorProvider =
-        getProvider(ClientTracingInterceptor.class);
-
-    Provider<HttpClient> httpClientProvider = () ->
-        // getProvider(HttpClient.class).get().copyWith(
-        Exceptions.uncheck(() -> HttpClient.of((s) -> {
-            ClientTracingInterceptor ic = clientTracingInterceptorProvider.get();
-            s.requestIntercept(ic::request);
-            s.responseIntercept(ic::response);
-            s.errorIntercept(ic::error);
-          })
-        );
+    OptionalBinder.newOptionalBinder(binder(), HttpClient.class)
+            .setDefault().toInstance(Exceptions.uncheck(() -> HttpClient.of(Action.noop())));
 
     bind(HttpClient.class)
         .annotatedWith(Zipkin.class)
-        .toProvider(httpClientProvider)
+        .toProvider(HttpClientProvider.class)
         .in(Singleton.class);
 
     bind(RatpackCurrentTraceContext.TracingPropagationExecInitializer.class)

src/main/java/ratpack/zipkin/internal/HttpClientProvider.java

@@ -0,0 +1,47 @@
+/*
+ * Copyright 2016-2019 The OpenZipkin Authors
+ *
+ * Licensed 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
+ *
+ * http://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 ratpack.zipkin.internal;
+
+import com.google.inject.Inject;
+import com.google.inject.Provider;
+import ratpack.http.client.HttpClient;
+import ratpack.util.Exceptions;
+import ratpack.zipkin.ClientTracingInterceptor;
+
+/**
+ * Provide the zipkin annotated http client,layered on top of a default
+ * or consumer provided http client.
+ */
+public class HttpClientProvider implements Provider<HttpClient> {
+
+    private HttpClient httpClient;
+    private ClientTracingInterceptor clientTracingInterceptor;
+
+    @Inject
+    public HttpClientProvider(HttpClient httpClient, ClientTracingInterceptor clientTracingInterceptor) {
+        this.clientTracingInterceptor = clientTracingInterceptor;
+        this.httpClient = httpClient;
+    }
+
+    @Override
+    public HttpClient get() {
+        return Exceptions.uncheck(() ->
+                httpClient.copyWith((s) -> {
+                    s.requestIntercept(clientTracingInterceptor::request);
+                    s.responseIntercept(clientTracingInterceptor::response);
+                    s.errorIntercept(clientTracingInterceptor::error);
+                }));
+    }
+
+}

src/test/groovy/ratpack/zipkin/ServerTracingModuleSpec.groovy

@@ -26,15 +26,17 @@ import ratpack.handling.Context
 import ratpack.handling.Handler
 import ratpack.http.HttpMethod
 import ratpack.http.client.HttpClient
+import ratpack.http.client.internal.DefaultHttpClient
 import ratpack.path.PathBinding
-
 import ratpack.zipkin.support.B3PropagationHeaders
 import ratpack.zipkin.support.TestReporter
 import spock.lang.Specification
 import spock.lang.Unroll
 import zipkin2.Span
 import zipkin2.reporter.Reporter
 
+import java.time.Duration
+
 import static org.assertj.core.api.Assertions.assertThat
 import static ratpack.groovy.test.embed.GroovyEmbeddedApp.ratpack
 
@@ -511,12 +513,9 @@ class ServerTracingModuleSpec extends Specification {
                     })
 				}
 				handlers { chain ->
-					chain.get("say/:message", new Handler() {
-						@Override
-						void handle(final Context ctx) throws Exception {
-							ctx.response.send("yo!")
-						}
-					})
+					chain.get("say/:message") { ctx ->
+						ctx.response.send("yo!")
+					}
                 }
             }
 		when:
@@ -623,4 +622,34 @@ class ServerTracingModuleSpec extends Specification {
 			assertThat(span.tags()).containsKey("after_render")
 			assertThat(span.tags()).containsValue("the_param")
 	}
-}
+
+	def "test that a consumer app can provide their own http client configs"() {
+		given:
+			def app = ratpack {
+				bindings {
+					module(ServerTracingModule.class)
+					bindInstance(HttpClient, HttpClient.of {
+						it.readTimeout(Duration.ofMillis(5000))
+						it.connectTimeout(Duration.ofMillis(5000))
+						it.poolSize(100)
+					})
+				}
+				handlers { chain ->
+					chain.get("say/:message") { ctx ->
+						ctx.response.send("yo!")
+					}
+				}
+			}
+		when:
+			app.test { t ->
+				t.get("say/hello")
+			}
+			def client = app.server.registry.get().get(HttpClient)
+		then:
+			client.readTimeout.toMillis() == 5000
+			client.connectTimeout.toMillis() == 5000
+			client.poolSize == 100
+			((DefaultHttpClient)client).requestInterceptor
+			((DefaultHttpClient)client).responseInterceptor
+	}
+}

src/test/java/brave/http/ITServerTracingModule.java

@@ -1,5 +1,5 @@
 /*
- * Copyright 2016-2018 The OpenZipkin Authors
+ * Copyright 2016-2019 The OpenZipkin Authors
  *
  * Licensed 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
@@ -35,28 +35,28 @@ protected void init() throws Exception {
         .of(server ->
             server.registry(Guice.registry(binding -> binding.module(tracingModule)))
                   .handlers(chain -> chain
-                      .options("/", ctx -> ctx.getResponse().send(""))
-                      .get("/foo", ctx -> ctx.getResponse().send("bar"))
-                      .get("/async", ctx ->
+                      .options("", ctx -> ctx.getResponse().send(""))
+                      .get("foo", ctx -> ctx.getResponse().send("bar"))
+                      .get("async", ctx ->
                           Promise.async(f -> f.success("bar")).then(ctx::render)
                       )
-                      .get("/badrequest", ctx -> ctx.getResponse().status(400).send())
-                      .get("/child", ctx -> {
+                      .get("badrequest", ctx -> ctx.getResponse().status(400).send())
+                      .get("child", ctx -> {
                         HttpTracing httpTracing = ctx.get(HttpTracing.class);
                         httpTracing.tracing().tracer().nextSpan().name("child").start().finish();
                         ctx.getResponse().send("happy");
                       })
-                      .get("/items/:itemId", ctx ->
+                      .get("items/:itemId", ctx ->
                           ctx.getResponse().send(ctx.getPathTokens().get("itemId"))
                       )
-                      .prefix("/nested", nested -> nested.get("items/:itemId", ctx ->
+                      .prefix("nested", nested -> nested.get("items/:itemId", ctx ->
                           ctx.getResponse().send(ctx.getPathTokens().get("itemId"))
                       ))
-                      .get("/extra", ctx -> ctx.getResponse().send("joey"))
-                      .get("/exception", ctx -> {
+                      .get("extra", ctx -> ctx.getResponse().send("joey"))
+                      .get("exception", ctx -> {
                         throw new IOException();
                       })
-                      .get("/exceptionAsync",
+                      .get("exceptionAsync",
                           ctx -> Promise.async((f) -> f.error(new IOException())).then(ctx::render)
                       )
                       .all(ctx -> ctx.getResponse().status(404).send()))
@@ -67,7 +67,7 @@ protected void init() throws Exception {
   protected String url(final String path) {
     URI uri = app.getAddress();
     return String
-        .format("%s://%s:%d/%s", uri.getScheme(), "127.0.0.1", uri.getPort(), path);
+        .format("%s://%s:%d/%s", uri.getScheme(), "127.0.0.1", uri.getPort(), path.replaceFirst("/", ""));
   }
 
 }