Properly implements old call-path of status codes

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;

instrumentation/http/src/test/java/brave/http/features/TracingInterceptor.java

@@ -68,8 +68,9 @@ static final class OkHttpAdapter extends brave.http.HttpClientAdapter<Request, R
       return request.header(name);
     }
 
-    @Override public Integer statusCode(Response response) {
-      return statusCodeAsInt(response);
+    @Override @Nullable public Integer statusCode(Response response) {
+      int result = statusCodeAsInt(response);
+      return result != 0 ? result : null;
     }
 
     @Override public int statusCodeAsInt(Response response) {

instrumentation/httpasyncclient/src/main/java/brave/httpasyncclient/TracingHttpAsyncClientBuilder.java

@@ -17,6 +17,7 @@
 import brave.Tracing;
 import brave.http.HttpClientHandler;
 import brave.http.HttpTracing;
+import brave.internal.Nullable;
 import brave.propagation.CurrentTraceContext;
 import brave.propagation.CurrentTraceContext.Scope;
 import brave.propagation.Propagation;
@@ -32,6 +33,7 @@
 import org.apache.http.HttpRequestInterceptor;
 import org.apache.http.HttpResponse;
 import org.apache.http.HttpResponseInterceptor;
+import org.apache.http.StatusLine;
 import org.apache.http.client.methods.HttpRequestWrapper;
 import org.apache.http.client.protocol.HttpClientContext;
 import org.apache.http.concurrent.FutureCallback;
@@ -157,12 +159,14 @@ static final class HttpAdapter extends brave.http.HttpClientAdapter<HttpRequest,
       return result != null ? result.getValue() : null;
     }
 
-    @Override public Integer statusCode(HttpResponse response) {
-      return statusCodeAsInt(response);
+    @Override @Nullable public Integer statusCode(HttpResponse response) {
+      int result = statusCodeAsInt(response);
+      return result != 0 ? result : null;
     }
 
     @Override public int statusCodeAsInt(HttpResponse response) {
-      return response.getStatusLine().getStatusCode();
+      StatusLine statusLine = response.getStatusLine();
+      return statusLine != null ? statusLine.getStatusCode() : 0;
     }
   }
 

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();
+  }
+}

instrumentation/httpclient/src/main/java/brave/httpclient/HttpAdapter.java

@@ -14,10 +14,12 @@
 package brave.httpclient;
 
 import brave.Span;
+import brave.internal.Nullable;
 import java.net.InetAddress;
 import org.apache.http.Header;
 import org.apache.http.HttpHost;
 import org.apache.http.HttpResponse;
+import org.apache.http.StatusLine;
 import org.apache.http.client.methods.HttpRequestWrapper;
 
 final class HttpAdapter extends brave.http.HttpClientAdapter<HttpRequestWrapper, HttpResponse> {
@@ -42,12 +44,14 @@ final class HttpAdapter extends brave.http.HttpClientAdapter<HttpRequestWrapper,
     return result != null ? result.getValue() : null;
   }
 
-  @Override public Integer statusCode(HttpResponse response) {
-    return statusCodeAsInt(response);
+  @Override @Nullable public Integer statusCode(HttpResponse response) {
+    int result = statusCodeAsInt(response);
+    return result != 0 ? result : null;
   }
 
   @Override public int statusCodeAsInt(HttpResponse response) {
-    return response.getStatusLine().getStatusCode();
+    StatusLine statusLine = response.getStatusLine();
+    return statusLine != null ? statusLine.getStatusCode() : 0;
   }
 
   static void parseTargetAddress(HttpRequestWrapper httpRequest, Span span) {

instrumentation/httpclient/src/test/java/brave/httpclient/HttpAdapterTest.java

@@ -16,20 +16,26 @@
 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;
 
 @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();
+  }
 }

instrumentation/jaxrs2/src/main/java/brave/jaxrs2/TracingClientFilter.java

@@ -19,6 +19,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 javax.annotation.Priority;
@@ -110,12 +111,14 @@ static final class HttpAdapter
       return request.getHeaderString(name);
     }
 
-    @Override public Integer statusCode(ClientResponseContext response) {
-      return statusCodeAsInt(response);
+    @Override @Nullable public Integer statusCode(ClientResponseContext response) {
+      int result = statusCodeAsInt(response);
+      return result != 0 ? result : null;
     }
 
     @Override public int statusCodeAsInt(ClientResponseContext response) {
-      return response.getStatus();
+      int result = response.getStatus();
+      return result != -1 ? result : 0;
     }
   }
 }

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();
+  }
+}

instrumentation/jersey-server/src/main/java/brave/jersey/server/TracingApplicationEventListener.java

@@ -18,6 +18,7 @@
 import brave.http.HttpServerAdapter;
 import brave.http.HttpServerHandler;
 import brave.http.HttpTracing;
+import brave.internal.Nullable;
 import brave.propagation.Propagation.Getter;
 import brave.propagation.TraceContext;
 import javax.inject.Inject;
@@ -101,8 +102,9 @@ static final class Adapter extends HttpServerAdapter<ContainerRequest, RequestEv
       return (String) event.getContainerRequest().getProperty("http.route");
     }
 
-    @Override public Integer statusCode(RequestEvent event) {
-      return statusCodeAsInt(event);
+    @Override @Nullable public Integer statusCode(RequestEvent response) {
+      int result = statusCodeAsInt(response);
+      return result != 0 ? result : null;
     }
 
     @Override public int statusCodeAsInt(RequestEvent event) {

instrumentation/jersey-server/src/test/java/brave/jersey/server/TracingApplicationEventListenerAdapterTest.java

@@ -40,35 +40,22 @@ 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() {
     when(event.getContainerRequest()).thenReturn(request);
     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();
   }
 }

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
+

instrumentation/netty-codec-http/src/main/java/brave/netty/http/HttpNettyAdapter.java

@@ -14,8 +14,10 @@
 package brave.netty.http;
 
 import brave.http.HttpServerAdapter;
+import brave.internal.Nullable;
 import io.netty.handler.codec.http.HttpRequest;
 import io.netty.handler.codec.http.HttpResponse;
+import io.netty.handler.codec.http.HttpResponseStatus;
 
 final class HttpNettyAdapter extends HttpServerAdapter<HttpRequest, HttpResponse> {
   @Override public String method(HttpRequest request) {
@@ -33,11 +35,13 @@ final class HttpNettyAdapter extends HttpServerAdapter<HttpRequest, HttpResponse
     return request.headers().get(name);
   }
 
-  @Override public Integer statusCode(HttpResponse response) {
-    return statusCodeAsInt(response);
+  @Override @Nullable public Integer statusCode(HttpResponse response) {
+    int result = statusCodeAsInt(response);
+    return result != 0 ? result : null;
   }
 
   @Override public int statusCodeAsInt(HttpResponse response) {
-    return response.status().code();
+    HttpResponseStatus status = response.status();
+    return status != null ? status.code() : 0;
   }
 }