From e4488ae65b1d504ca5307cb4d9b4db6c1c78a722 Mon Sep 17 00:00:00 2001 From: Adrian Cole Date: Wed, 3 Jul 2019 15:59:04 +0800 Subject: [PATCH 1/2] WIP properly implements old call-path of status codes Eventhough new instrumentation don't use it, old could. See https://github.com/spring-cloud/spring-cloud-sleuth/pull/1387 --- .../brave/http/features/TracingInterceptor.java | 5 +++-- .../TracingHttpAsyncClientBuilder.java | 10 +++++++--- .../main/java/brave/httpclient/HttpAdapter.java | 10 +++++++--- .../java/brave/jaxrs2/TracingClientFilter.java | 9 ++++++--- .../server/TracingApplicationEventListener.java | 6 ++++-- .../java/brave/netty/http/HttpNettyAdapter.java | 10 +++++++--- .../java/brave/okhttp3/TracingInterceptor.java | 6 ++++-- .../java/brave/servlet/HttpServletAdapter.java | 8 +++++++- .../main/java/brave/servlet/ServletRuntime.java | 17 ++++++++--------- .../TracingClientHttpRequestInterceptor.java | 12 +++++++++--- .../brave/vertx/web/VertxHttpServerAdapter.java | 6 ++++-- 11 files changed, 66 insertions(+), 33 deletions(-) diff --git a/instrumentation/http/src/test/java/brave/http/features/TracingInterceptor.java b/instrumentation/http/src/test/java/brave/http/features/TracingInterceptor.java index 15b5a4ea8f..1560b8f92b 100644 --- a/instrumentation/http/src/test/java/brave/http/features/TracingInterceptor.java +++ b/instrumentation/http/src/test/java/brave/http/features/TracingInterceptor.java @@ -68,8 +68,9 @@ static final class OkHttpAdapter extends brave.http.HttpClientAdapter { @@ -42,12 +44,14 @@ final class HttpAdapter extends brave.http.HttpClientAdapter { @Override public String method(HttpRequest request) { @@ -33,11 +35,13 @@ final class HttpNettyAdapter extends HttpServerAdapter, Object> replacement = new LinkedHashMap<>(classesToCheck); @@ -212,7 +211,7 @@ static final class Servlet25 extends ServletRuntime { Map, Object> replacement = new LinkedHashMap<>(classesToCheck); replacement.put(clazz, RETURN_NULL); classToGetStatus.set(replacement); // prefer overriding on failure - return null; + return 0; } } } @@ -246,7 +245,7 @@ static final class Servlet25ServerResponseAdapter extends HttpServletResponseWra super.setStatus(sc); } - public int getStatusInServlet25() { + int getStatusInServlet25() { return httpStatus; } } diff --git a/instrumentation/spring-web/src/main/java/brave/spring/web/TracingClientHttpRequestInterceptor.java b/instrumentation/spring-web/src/main/java/brave/spring/web/TracingClientHttpRequestInterceptor.java index f0d9f37074..3d82ff2a8c 100644 --- a/instrumentation/spring-web/src/main/java/brave/spring/web/TracingClientHttpRequestInterceptor.java +++ b/instrumentation/spring-web/src/main/java/brave/spring/web/TracingClientHttpRequestInterceptor.java @@ -18,6 +18,7 @@ import brave.Tracing; import brave.http.HttpClientHandler; import brave.http.HttpTracing; +import brave.internal.Nullable; import brave.propagation.Propagation.Setter; import brave.propagation.TraceContext; import java.io.IOException; @@ -88,11 +89,16 @@ static final class HttpAdapter return result != null ? result.toString() : ""; } - @Override public Integer statusCode(ClientHttpResponse response) { + @Override @Nullable public Integer statusCode(ClientHttpResponse response) { + int result = statusCodeAsInt(response); + return result != 0 ? result : null; + } + + @Override public int statusCodeAsInt(ClientHttpResponse response) { try { return response.getRawStatusCode(); - } catch (IOException e) { - return null; + } catch (IllegalArgumentException | IOException e) { + return 0; } } } diff --git a/instrumentation/vertx-web/src/main/java/brave/vertx/web/VertxHttpServerAdapter.java b/instrumentation/vertx-web/src/main/java/brave/vertx/web/VertxHttpServerAdapter.java index 32217bb381..10e96c4192 100644 --- a/instrumentation/vertx-web/src/main/java/brave/vertx/web/VertxHttpServerAdapter.java +++ b/instrumentation/vertx-web/src/main/java/brave/vertx/web/VertxHttpServerAdapter.java @@ -15,6 +15,7 @@ import brave.Span; import brave.http.HttpServerAdapter; +import brave.internal.Nullable; import io.vertx.core.http.HttpServerRequest; import io.vertx.core.http.HttpServerResponse; import io.vertx.core.net.SocketAddress; @@ -61,8 +62,9 @@ class VertxHttpServerAdapter extends HttpServerAdapter Date: Wed, 3 Jul 2019 16:49:10 +0800 Subject: [PATCH 2/2] and now with tests --- .../java/brave/SpanCustomizerShieldTest.java | 13 +++++ .../httpasyncclient/HttpAdapterTest.java | 45 ++++++++++++++++ .../brave/httpclient/HttpAdapterTest.java | 19 +++++++ .../java/brave/jaxrs2/HttpAdapterTest.java | 44 ++++++++++++++++ ...ngApplicationEventListenerAdapterTest.java | 34 ++++++------ .../src/test/resources/log4j2.properties | 9 ++++ .../netty/http/HttpNettyAdapterTest.java | 44 ++++++++++++++++ .../java/brave/okhttp3/HttpAdapterTest.java | 43 +++++++++++++++ .../brave/servlet/ServletRuntime25Test.java | 2 +- .../java/brave/servlet/ServletRuntime.java | 2 +- .../brave/servlet/HttpServletAdapterTest.java | 20 +++++-- .../brave/servlet/ServletRuntimeTest.java | 8 +-- .../brave/spring/web/HttpAdapterTest.java | 52 +++++++++++++++++++ .../vertx/web/VertxHttpServerAdapterTest.java | 40 +++++++------- 14 files changed, 329 insertions(+), 46 deletions(-) create mode 100644 instrumentation/httpasyncclient/src/test/java/brave/httpasyncclient/HttpAdapterTest.java create mode 100644 instrumentation/jaxrs2/src/test/java/brave/jaxrs2/HttpAdapterTest.java create mode 100755 instrumentation/jms/src/it/jms11/src/test/resources/log4j2.properties create mode 100644 instrumentation/netty-codec-http/src/test/java/brave/netty/http/HttpNettyAdapterTest.java create mode 100644 instrumentation/okhttp3/src/test/java/brave/okhttp3/HttpAdapterTest.java create mode 100644 instrumentation/spring-web/src/test/java/brave/spring/web/HttpAdapterTest.java diff --git a/brave/src/test/java/brave/SpanCustomizerShieldTest.java b/brave/src/test/java/brave/SpanCustomizerShieldTest.java index 349cca6fa9..3075820483 100644 --- a/brave/src/test/java/brave/SpanCustomizerShieldTest.java +++ b/brave/src/test/java/brave/SpanCustomizerShieldTest.java @@ -1,3 +1,16 @@ +/* + * Copyright 2013-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 brave; import org.junit.Test; diff --git a/instrumentation/httpasyncclient/src/test/java/brave/httpasyncclient/HttpAdapterTest.java b/instrumentation/httpasyncclient/src/test/java/brave/httpasyncclient/HttpAdapterTest.java new file mode 100644 index 0000000000..e958f27b20 --- /dev/null +++ b/instrumentation/httpasyncclient/src/test/java/brave/httpasyncclient/HttpAdapterTest.java @@ -0,0 +1,45 @@ +/* + * Copyright 2013-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 brave.httpasyncclient; + +import brave.httpasyncclient.TracingHttpAsyncClientBuilder.HttpAdapter; +import org.apache.http.HttpResponse; +import org.apache.http.StatusLine; +import org.junit.Test; +import org.junit.runner.RunWith; +import org.mockito.Mock; +import org.mockito.junit.MockitoJUnitRunner; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.mockito.Mockito.when; + +@RunWith(MockitoJUnitRunner.class) +public class HttpAdapterTest { + @Mock HttpResponse response; + @Mock StatusLine statusLine; + HttpAdapter adapter = new HttpAdapter(); + + @Test public void statusCodeAsInt() { + when(response.getStatusLine()).thenReturn(statusLine); + when(statusLine.getStatusCode()).thenReturn(200); + + assertThat(adapter.statusCodeAsInt(response)).isEqualTo(200); + assertThat(adapter.statusCode(response)).isEqualTo(200); + } + + @Test public void statusCodeAsInt_zeroWhenNoStatusLine() { + assertThat(adapter.statusCodeAsInt(response)).isZero(); + assertThat(adapter.statusCode(response)).isNull(); + } +} diff --git a/instrumentation/httpclient/src/test/java/brave/httpclient/HttpAdapterTest.java b/instrumentation/httpclient/src/test/java/brave/httpclient/HttpAdapterTest.java index 718481ff01..943b68082d 100644 --- a/instrumentation/httpclient/src/test/java/brave/httpclient/HttpAdapterTest.java +++ b/instrumentation/httpclient/src/test/java/brave/httpclient/HttpAdapterTest.java @@ -16,12 +16,15 @@ import java.net.InetAddress; import java.net.UnknownHostException; import org.apache.http.HttpHost; +import org.apache.http.HttpResponse; +import org.apache.http.StatusLine; import org.apache.http.client.methods.HttpRequestWrapper; import org.junit.Test; import org.junit.runner.RunWith; import org.mockito.Mock; import org.mockito.junit.MockitoJUnitRunner; +import static org.assertj.core.api.Assertions.assertThat; import static org.mockito.Mockito.verify; import static org.mockito.Mockito.verifyNoMoreInteractions; import static org.mockito.Mockito.when; @@ -29,7 +32,10 @@ @RunWith(MockitoJUnitRunner.class) public class HttpAdapterTest { @Mock HttpRequestWrapper request; + @Mock HttpResponse response; + @Mock StatusLine statusLine; @Mock brave.Span span; + HttpAdapter adapter = new HttpAdapter(); @Test public void parseTargetAddress_skipsOnNoop() { when(span.isNoop()).thenReturn(true); @@ -76,4 +82,17 @@ public class HttpAdapterTest { verify(span).remoteIpAndPort("1.2.3.4", 9999); verifyNoMoreInteractions(span); } + + @Test public void statusCodeAsInt() { + when(response.getStatusLine()).thenReturn(statusLine); + when(statusLine.getStatusCode()).thenReturn(200); + + assertThat(adapter.statusCodeAsInt(response)).isEqualTo(200); + assertThat(adapter.statusCode(response)).isEqualTo(200); + } + + @Test public void statusCodeAsInt_zeroWhenNoStatusLine() { + assertThat(adapter.statusCodeAsInt(response)).isZero(); + assertThat(adapter.statusCode(response)).isNull(); + } } diff --git a/instrumentation/jaxrs2/src/test/java/brave/jaxrs2/HttpAdapterTest.java b/instrumentation/jaxrs2/src/test/java/brave/jaxrs2/HttpAdapterTest.java new file mode 100644 index 0000000000..deab07d5bf --- /dev/null +++ b/instrumentation/jaxrs2/src/test/java/brave/jaxrs2/HttpAdapterTest.java @@ -0,0 +1,44 @@ +/* + * Copyright 2013-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 brave.jaxrs2; + +import brave.jaxrs2.TracingClientFilter.HttpAdapter; +import javax.ws.rs.client.ClientResponseContext; +import org.junit.Test; +import org.junit.runner.RunWith; +import org.mockito.Mock; +import org.mockito.junit.MockitoJUnitRunner; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.mockito.Mockito.when; + +@RunWith(MockitoJUnitRunner.class) +public class HttpAdapterTest { + @Mock ClientResponseContext response; + HttpAdapter adapter = new HttpAdapter(); + + @Test public void statusCodeAsInt() { + when(response.getStatus()).thenReturn(200); + + assertThat(adapter.statusCodeAsInt(response)).isEqualTo(200); + assertThat(adapter.statusCode(response)).isEqualTo(200); + } + + @Test public void statusCodeAsInt_zeroWhenNegative() { + when(response.getStatus()).thenReturn(-1); + + assertThat(adapter.statusCodeAsInt(response)).isZero(); + assertThat(adapter.statusCode(response)).isNull(); + } +} diff --git a/instrumentation/jersey-server/src/test/java/brave/jersey/server/TracingApplicationEventListenerAdapterTest.java b/instrumentation/jersey-server/src/test/java/brave/jersey/server/TracingApplicationEventListenerAdapterTest.java index b29e69b8f8..ed20087fde 100644 --- a/instrumentation/jersey-server/src/test/java/brave/jersey/server/TracingApplicationEventListenerAdapterTest.java +++ b/instrumentation/jersey-server/src/test/java/brave/jersey/server/TracingApplicationEventListenerAdapterTest.java @@ -40,14 +40,14 @@ public class TracingApplicationEventListenerAdapterTest { when(request.getMethod()).thenReturn("GET"); assertThat(adapter.methodFromResponse(event)) - .isEqualTo("GET"); + .isEqualTo("GET"); } @Test public void path_prefixesSlashWhenMissing() { when(request.getPath(false)).thenReturn("bar"); assertThat(adapter.path(request)) - .isEqualTo("/bar"); + .isEqualTo("/bar"); } @Test public void route() { @@ -55,20 +55,7 @@ public class TracingApplicationEventListenerAdapterTest { when(request.getProperty("http.route")).thenReturn("/items/{itemId}"); assertThat(adapter.route(event)) - .isEqualTo("/items/{itemId}"); - } - - @Test public void statusCodeAsInt() { - when(event.getContainerResponse()).thenReturn(response); - when(response.getStatus()).thenReturn(200); - - assertThat(adapter.statusCodeAsInt(event)) - .isEqualTo(200); - } - - @Test public void statusCodeAsInt_noResponse() { - assertThat(adapter.statusCodeAsInt(event)) - .isZero(); + .isEqualTo("/items/{itemId}"); } @Test public void url_derivedFromExtendedUriInfo() { @@ -77,6 +64,19 @@ public class TracingApplicationEventListenerAdapterTest { when(uriInfo.getRequestUri()).thenReturn(URI.create("http://foo:8080/bar?hello=world")); assertThat(adapter.url(request)) - .isEqualTo("http://foo:8080/bar?hello=world"); + .isEqualTo("http://foo:8080/bar?hello=world"); + } + + @Test public void statusCodeAsInt() { + when(event.getContainerResponse()).thenReturn(response); + when(response.getStatus()).thenReturn(200); + + assertThat(adapter.statusCodeAsInt(event)).isEqualTo(200); + assertThat(adapter.statusCode(event)).isEqualTo(200); + } + + @Test public void statusCodeAsInt_zeroNoResponse() { + assertThat(adapter.statusCodeAsInt(event)).isZero(); + assertThat(adapter.statusCode(event)).isNull(); } } diff --git a/instrumentation/jms/src/it/jms11/src/test/resources/log4j2.properties b/instrumentation/jms/src/it/jms11/src/test/resources/log4j2.properties new file mode 100755 index 0000000000..e6988c4497 --- /dev/null +++ b/instrumentation/jms/src/it/jms11/src/test/resources/log4j2.properties @@ -0,0 +1,9 @@ +appenders=console +appender.console.type=Console +appender.console.name=STDOUT +appender.console.layout.type=PatternLayout +appender.console.layout.pattern=%d{ABSOLUTE} %-5p [%t] %C{2} (%F:%L) - %m%n +rootLogger.level=warn +rootLogger.appenderRefs=stdout +rootLogger.appenderRef.stdout.ref=STDOUT + diff --git a/instrumentation/netty-codec-http/src/test/java/brave/netty/http/HttpNettyAdapterTest.java b/instrumentation/netty-codec-http/src/test/java/brave/netty/http/HttpNettyAdapterTest.java new file mode 100644 index 0000000000..85a1a9e8da --- /dev/null +++ b/instrumentation/netty-codec-http/src/test/java/brave/netty/http/HttpNettyAdapterTest.java @@ -0,0 +1,44 @@ +/* + * Copyright 2013-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 brave.netty.http; + +import io.netty.handler.codec.http.HttpResponse; +import io.netty.handler.codec.http.HttpResponseStatus; +import org.junit.Test; +import org.junit.runner.RunWith; +import org.mockito.Mock; +import org.mockito.junit.MockitoJUnitRunner; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.mockito.Mockito.when; + +@RunWith(MockitoJUnitRunner.class) +public class HttpNettyAdapterTest { + @Mock HttpResponse response; + @Mock HttpResponseStatus status; + HttpNettyAdapter adapter = new HttpNettyAdapter(); + + @Test public void statusCodeAsInt() { + when(response.status()).thenReturn(status); + when(status.code()).thenReturn(200); + + assertThat(adapter.statusCodeAsInt(response)).isEqualTo(200); + assertThat(adapter.statusCode(response)).isEqualTo(200); + } + + @Test public void statusCodeAsInt_zeroNoResponse() { + assertThat(adapter.statusCodeAsInt(response)).isZero(); + assertThat(adapter.statusCode(response)).isNull(); + } +} diff --git a/instrumentation/okhttp3/src/test/java/brave/okhttp3/HttpAdapterTest.java b/instrumentation/okhttp3/src/test/java/brave/okhttp3/HttpAdapterTest.java new file mode 100644 index 0000000000..d4d7613fb2 --- /dev/null +++ b/instrumentation/okhttp3/src/test/java/brave/okhttp3/HttpAdapterTest.java @@ -0,0 +1,43 @@ +/* + * Copyright 2013-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 brave.okhttp3; + +import brave.okhttp3.TracingInterceptor.HttpAdapter; +import okhttp3.Protocol; +import okhttp3.Request; +import okhttp3.Response; +import org.junit.Test; + +import static org.assertj.core.api.Assertions.assertThat; + +public class HttpAdapterTest { + HttpAdapter adapter = new HttpAdapter(); + Response.Builder responseBuilder = new Response.Builder() + .request(new Request.Builder().url("http://localhost/foo").build()) + .protocol(Protocol.HTTP_1_1); + + @Test public void statusCodeAsInt() { + Response response = responseBuilder.code(200).message("ok").build(); + + assertThat(adapter.statusCodeAsInt(response)).isEqualTo(200); + assertThat(adapter.statusCode(response)).isEqualTo(200); + } + + @Test public void statusCodeAsInt_zero() { + Response response = responseBuilder.code(0).message("ice cream!").build(); + + assertThat(adapter.statusCodeAsInt(response)).isZero(); + assertThat(adapter.statusCode(response)).isNull(); + } +} diff --git a/instrumentation/servlet/src/it/servlet25/src/test/java/brave/servlet/ServletRuntime25Test.java b/instrumentation/servlet/src/it/servlet25/src/test/java/brave/servlet/ServletRuntime25Test.java index af4ca72c9f..9b73785fb5 100644 --- a/instrumentation/servlet/src/it/servlet25/src/test/java/brave/servlet/ServletRuntime25Test.java +++ b/instrumentation/servlet/src/it/servlet25/src/test/java/brave/servlet/ServletRuntime25Test.java @@ -45,7 +45,7 @@ public class ServletRuntime25Test { @Test public void status_fromInvalidMethod() throws Exception { assertThat(ServletRuntime.get().status(new WithInvalidGetStatus())) - .isNull(); + .isZero(); } public static class WithInvalidGetStatus extends WithoutGetStatus { diff --git a/instrumentation/servlet/src/main/java/brave/servlet/ServletRuntime.java b/instrumentation/servlet/src/main/java/brave/servlet/ServletRuntime.java index 6e17c9a35d..0bf8335213 100644 --- a/instrumentation/servlet/src/main/java/brave/servlet/ServletRuntime.java +++ b/instrumentation/servlet/src/main/java/brave/servlet/ServletRuntime.java @@ -245,7 +245,7 @@ static final class Servlet25ServerResponseAdapter extends HttpServletResponseWra super.setStatus(sc); } - int getStatusInServlet25() { + public int getStatusInServlet25() { return httpStatus; } } diff --git a/instrumentation/servlet/src/test/java/brave/servlet/HttpServletAdapterTest.java b/instrumentation/servlet/src/test/java/brave/servlet/HttpServletAdapterTest.java index 1b072e5c31..e309e9d157 100644 --- a/instrumentation/servlet/src/test/java/brave/servlet/HttpServletAdapterTest.java +++ b/instrumentation/servlet/src/test/java/brave/servlet/HttpServletAdapterTest.java @@ -15,6 +15,7 @@ import brave.Span; import javax.servlet.http.HttpServletRequest; +import javax.servlet.http.HttpServletResponse; import org.junit.Test; import org.junit.runner.RunWith; import org.mockito.Mock; @@ -29,18 +30,19 @@ public class HttpServletAdapterTest { HttpServletAdapter adapter = new HttpServletAdapter(); @Mock HttpServletRequest request; + @Mock HttpServletResponse response; @Mock Span span; @Test public void path_doesntCrashOnNullUrl() { assertThat(adapter.path(request)) - .isNull(); + .isNull(); } @Test public void path_getRequestURI() { when(request.getRequestURI()).thenReturn("/bar"); assertThat(adapter.path(request)) - .isEqualTo("/bar"); + .isEqualTo("/bar"); } @Test public void url_derivedFromUrlAndQueryString() { @@ -48,7 +50,7 @@ public class HttpServletAdapterTest { when(request.getQueryString()).thenReturn("hello=world"); assertThat(adapter.url(request)) - .isEqualTo("http://foo:8080/bar?hello=world"); + .isEqualTo("http://foo:8080/bar?hello=world"); } @Test public void parseClientIpAndPort_prefersXForwardedFor() { @@ -80,4 +82,16 @@ public class HttpServletAdapterTest { verify(span).remoteIpAndPort("1.2.3.4", 61687); verifyNoMoreInteractions(span); } + + @Test public void statusCodeAsInt() { + when(response.getStatus()).thenReturn(200); + + assertThat(adapter.statusCodeAsInt(response)).isEqualTo(200); + assertThat(adapter.statusCode(response)).isEqualTo(200); + } + + @Test public void statusCodeAsInt_zeroNoResponse() { + assertThat(adapter.statusCodeAsInt(response)).isZero(); + assertThat(adapter.statusCode(response)).isNull(); + } } diff --git a/instrumentation/servlet/src/test/java/brave/servlet/ServletRuntimeTest.java b/instrumentation/servlet/src/test/java/brave/servlet/ServletRuntimeTest.java index 4ed89ea9ed..b038bcf8c6 100644 --- a/instrumentation/servlet/src/test/java/brave/servlet/ServletRuntimeTest.java +++ b/instrumentation/servlet/src/test/java/brave/servlet/ServletRuntimeTest.java @@ -35,7 +35,7 @@ public class ServletRuntimeTest { } }; assertThat(servlet25.status(jettyResponse)) - .isNull(); + .isZero(); } @Test public void servlet25_status_doesntParseLocalTypes() throws Exception { @@ -43,7 +43,7 @@ public class ServletRuntimeTest { class LocalResponse extends HttpServletResponseImpl { } assertThat(servlet25.status(new LocalResponse())) - .isNull(); + .isZero(); } class ExceptionResponse extends HttpServletResponseImpl { @@ -54,7 +54,7 @@ class ExceptionResponse extends HttpServletResponseImpl { @Test public void servlet25_status_nullOnException() throws Exception { assertThat(servlet25.status(new ExceptionResponse())) - .isNull(); + .isZero(); } class Response1 extends HttpServletResponseImpl { @@ -112,7 +112,7 @@ class Response11 extends HttpServletResponseImpl { assertThat(servlet25.status(new Response10())) .isEqualTo(200); assertThat(servlet25.status(new Response11())) - .isNull(); + .isZero(); } public static class HttpServletResponseImpl implements HttpServletResponse { diff --git a/instrumentation/spring-web/src/test/java/brave/spring/web/HttpAdapterTest.java b/instrumentation/spring-web/src/test/java/brave/spring/web/HttpAdapterTest.java new file mode 100644 index 0000000000..558eb35897 --- /dev/null +++ b/instrumentation/spring-web/src/test/java/brave/spring/web/HttpAdapterTest.java @@ -0,0 +1,52 @@ +/* + * Copyright 2013-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 brave.spring.web; + +import java.io.IOException; +import org.junit.Test; +import org.junit.runner.RunWith; +import org.mockito.Mock; +import org.mockito.junit.MockitoJUnitRunner; +import org.springframework.http.client.ClientHttpResponse; + +import static brave.spring.web.TracingClientHttpRequestInterceptor.HttpAdapter; +import static org.assertj.core.api.Assertions.assertThat; +import static org.mockito.Mockito.when; + +@RunWith(MockitoJUnitRunner.class) +public class HttpAdapterTest { + @Mock ClientHttpResponse response; + HttpAdapter adapter = new HttpAdapter(); + + @Test public void statusCodeAsInt() throws IOException { + when(response.getRawStatusCode()).thenReturn(200); + + assertThat(adapter.statusCodeAsInt(response)).isEqualTo(200); + assertThat(adapter.statusCode(response)).isEqualTo(200); + } + + @Test public void statusCodeAsInt_zeroOnIOE() throws IOException { + when(response.getRawStatusCode()).thenThrow(new IOException()); + + assertThat(adapter.statusCodeAsInt(response)).isZero(); + assertThat(adapter.statusCode(response)).isNull(); + } + + @Test public void statusCodeAsInt_zeroOnIAE() throws IOException { + when(response.getRawStatusCode()).thenThrow(new IllegalArgumentException()); + + assertThat(adapter.statusCodeAsInt(response)).isZero(); + assertThat(adapter.statusCode(response)).isNull(); + } +} diff --git a/instrumentation/vertx-web/src/test/java/brave/vertx/web/VertxHttpServerAdapterTest.java b/instrumentation/vertx-web/src/test/java/brave/vertx/web/VertxHttpServerAdapterTest.java index 11d71c76af..99371a3aa8 100644 --- a/instrumentation/vertx-web/src/test/java/brave/vertx/web/VertxHttpServerAdapterTest.java +++ b/instrumentation/vertx-web/src/test/java/brave/vertx/web/VertxHttpServerAdapterTest.java @@ -14,14 +14,19 @@ package brave.vertx.web; import io.vertx.core.http.HttpServerResponse; -import java.lang.reflect.Proxy; import org.junit.After; import org.junit.Test; +import org.junit.runner.RunWith; +import org.mockito.Mock; +import org.mockito.junit.MockitoJUnitRunner; import static brave.test.util.ClassLoaders.assertRunIsUnloadable; import static org.assertj.core.api.Assertions.assertThat; +import static org.mockito.Mockito.when; +@RunWith(MockitoJUnitRunner.class) public class VertxHttpServerAdapterTest { + @Mock HttpServerResponse response; VertxHttpServerAdapter adapter = new VertxHttpServerAdapter(); @After public void clear() { @@ -29,35 +34,41 @@ public class VertxHttpServerAdapterTest { } @Test public void methodFromResponse() { - HttpServerResponse response = dummyResponse(); - VertxHttpServerAdapter.setCurrentMethodAndPath("GET", null); assertThat(adapter.methodFromResponse(response)) - .isEqualTo("GET"); + .isEqualTo("GET"); } @Test public void route_emptyByDefault() { - HttpServerResponse response = dummyResponse(); - VertxHttpServerAdapter.setCurrentMethodAndPath("GET", null); assertThat(adapter.route(response)).isEmpty(); } @Test public void route() { - HttpServerResponse response = dummyResponse(); - VertxHttpServerAdapter.setCurrentMethodAndPath("GET", "/users/:userID"); assertThat(adapter.route(response)) - .isEqualTo("/users/:userID"); + .isEqualTo("/users/:userID"); } @Test public void setCurrentMethodAndPath_doesntPreventClassUnloading() { assertRunIsUnloadable(MethodFromResponse.class, getClass().getClassLoader()); } + @Test public void statusCodeAsInt() { + when(response.getStatusCode()).thenReturn(200); + + assertThat(adapter.statusCodeAsInt(response)).isEqualTo(200); + assertThat(adapter.statusCode(response)).isEqualTo(200); + } + + @Test public void statusCodeAsInt_zero() { + assertThat(adapter.statusCodeAsInt(response)).isZero(); + assertThat(adapter.statusCode(response)).isNull(); + } + static class MethodFromResponse implements Runnable { final VertxHttpServerAdapter adapter = new VertxHttpServerAdapter(); @@ -66,15 +77,4 @@ static class MethodFromResponse implements Runnable { adapter.methodFromResponse(null); } } - - /** In JRE 1.8, mockito crashes with 'Mockito cannot mock this class' */ - HttpServerResponse dummyResponse() { - return (HttpServerResponse) Proxy.newProxyInstance( - getClass().getClassLoader(), - new Class[] {HttpServerResponse.class}, - (proxy, method, methodArgs) -> { - throw new UnsupportedOperationException( - "Unsupported method: " + method.getName()); - }); - } }