diff --git a/src/main/java/ratpack/zipkin/ServerTracingModule.java b/src/main/java/ratpack/zipkin/ServerTracingModule.java index 47f3bd0..e4e6949 100644 --- a/src/main/java/ratpack/zipkin/ServerTracingModule.java +++ b/src/main/java/ratpack/zipkin/ServerTracingModule.java @@ -28,6 +28,8 @@ 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; @@ -35,6 +37,7 @@ 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 clientTracingInterceptorProvider = - getProvider(ClientTracingInterceptor.class); - - Provider 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) diff --git a/src/main/java/ratpack/zipkin/internal/HttpClientProvider.java b/src/main/java/ratpack/zipkin/internal/HttpClientProvider.java new file mode 100644 index 0000000..ba52f9a --- /dev/null +++ b/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 { + + 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); + })); + } + +} diff --git a/src/test/groovy/ratpack/zipkin/ServerTracingModuleSpec.groovy b/src/test/groovy/ratpack/zipkin/ServerTracingModuleSpec.groovy index 63990ee..a441620 100644 --- a/src/test/groovy/ratpack/zipkin/ServerTracingModuleSpec.groovy +++ b/src/test/groovy/ratpack/zipkin/ServerTracingModuleSpec.groovy @@ -26,8 +26,8 @@ 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 @@ -35,6 +35,8 @@ 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") } -} \ No newline at end of file + + 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 + } +} diff --git a/src/test/java/brave/http/ITServerTracingModule.java b/src/test/java/brave/http/ITServerTracingModule.java index 9635310..305d083 100644 --- a/src/test/java/brave/http/ITServerTracingModule.java +++ b/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("/", "")); } }