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/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; } } } 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/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/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/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 { - throw new UnsupportedOperationException( - "Unsupported method: " + method.getName()); - }); - } }