Fix a bunch of errorprone warnings. (#1806)

Bogdan Drutu · web-flow · commit d5ba9ce171bc · 2019-03-19T16:24:36.000-07:00
diff --git a/api/src/test/java/io/opencensus/common/ServerStatsEncodingTest.java b/api/src/test/java/io/opencensus/common/ServerStatsEncodingTest.java
@@ -34,9 +34,9 @@ public class ServerStatsEncodingTest {
 
   @Test
   public void encodeDecodeTest() throws ServerStatsDeserializationException {
-    ServerStats serverStatsToBeEncoded = null;
-    ServerStats serverStatsDecoded = null;
-    byte[] serialized = null;
+    ServerStats serverStatsToBeEncoded;
+    ServerStats serverStatsDecoded;
+    byte[] serialized;
 
     serverStatsToBeEncoded = ServerStats.create(31, 22, (byte) 1);
     serialized = ServerStatsEncoding.toBytes(serverStatsToBeEncoded);
@@ -56,9 +56,9 @@ public void encodeDecodeTest() throws ServerStatsDeserializationException {
 
   @Test
   public void skipUnknownFieldTest() throws ServerStatsDeserializationException {
-    ServerStats serverStatsToBeEncoded = null;
-    ServerStats serverStatsDecoded = null;
-    byte[] serialized = null;
+    ServerStats serverStatsToBeEncoded;
+    ServerStats serverStatsDecoded;
+    byte[] serialized;
 
     serverStatsToBeEncoded = ServerStats.create(31, 22, (byte) 1);
     serialized = ServerStatsEncoding.toBytes(serverStatsToBeEncoded);
@@ -79,8 +79,8 @@ public void skipUnknownFieldTest() throws ServerStatsDeserializationException {
 
   @Test
   public void negativeLbLatencyValueTest() throws ServerStatsDeserializationException {
-    ServerStats serverStatsToBeEncoded = null;
-    byte[] serialized = null;
+    ServerStats serverStatsToBeEncoded;
+    byte[] serialized;
 
     serverStatsToBeEncoded = ServerStats.create(31, 22, (byte) 1);
     serialized = ServerStatsEncoding.toBytes(serverStatsToBeEncoded);
@@ -99,8 +99,8 @@ public void negativeLbLatencyValueTest() throws ServerStatsDeserializationExcept
 
   @Test
   public void negativeServerLatencyValueTest() throws ServerStatsDeserializationException {
-    ServerStats serverStatsToBeEncoded = null;
-    byte[] serialized = null;
+    ServerStats serverStatsToBeEncoded;
+    byte[] serialized;
 
     serverStatsToBeEncoded = ServerStats.create(31, 22, (byte) 1);
     serialized = ServerStatsEncoding.toBytes(serverStatsToBeEncoded);
diff --git a/api/src/test/java/io/opencensus/common/ServerStatsTest.java b/api/src/test/java/io/opencensus/common/ServerStatsTest.java
@@ -32,7 +32,7 @@ public class ServerStatsTest {
 
   @Test
   public void serverStatsCreate() {
-    ServerStats serverStats = null;
+    ServerStats serverStats;
 
     serverStats = ServerStats.create(31, 22, (byte) 0);
     assertThat(serverStats.getLbLatencyNs()).isEqualTo(31);
diff --git a/api/src/test/java/io/opencensus/metrics/MetricRegistryTest.java b/api/src/test/java/io/opencensus/metrics/MetricRegistryTest.java
@@ -31,16 +31,16 @@
 public class MetricRegistryTest {
   @Rule public ExpectedException thrown = ExpectedException.none();
 
-  private static final String NAME = "name";
-  private static final String NAME_2 = "name2";
-  private static final String NAME_3 = "name3";
-  private static final String NAME_4 = "name4";
-  private static final String DESCRIPTION = "description";
+  private static final String NAME = "test_name";
+  private static final String NAME_2 = "test_name2";
+  private static final String NAME_3 = "test_name3";
+  private static final String NAME_4 = "test_name4";
+  private static final String DESCRIPTION = "test_description";
   private static final String UNIT = "1";
   private static final List<LabelKey> LABEL_KEY =
-      Collections.singletonList(LabelKey.create("key", "key description"));
+      Collections.singletonList(LabelKey.create("test_key", "test key description"));
   private static final List<LabelValue> LABEL_VALUES =
-      Collections.singletonList(LabelValue.create("value"));
+      Collections.singletonList(LabelValue.create("test_value"));
   private final MetricRegistry metricRegistry =
       MetricsComponent.newNoopMetricsComponent().getMetricRegistry();
 
diff --git a/contrib/http_jaxrs/src/test/java/io/opencensus/contrib/http/jaxrs/JaxrsClientFilterTest.java b/contrib/http_jaxrs/src/test/java/io/opencensus/contrib/http/jaxrs/JaxrsClientFilterTest.java
@@ -67,7 +67,7 @@ public void testResponseFilter() throws Exception {
     filter.filter(requestContext, responseContext);
 
     verify(requestContext).getProperty("opencensus.context");
-    verify(responseContext, times(2)).getStatus();
+    verify(responseContext, times(1)).getStatus();
   }
 
   static HttpRequestContext createHttpRequestContext(Span span, TagContext tagContext)
diff --git a/contrib/http_jaxrs/src/test/java/io/opencensus/contrib/http/jaxrs/JaxrsContainerFilterTest.java b/contrib/http_jaxrs/src/test/java/io/opencensus/contrib/http/jaxrs/JaxrsContainerFilterTest.java
@@ -72,6 +72,6 @@ public void testResponseFilter() throws Exception {
     ContainerResponseContext responseContext = mock(ContainerResponseContext.class);
     filter.filter(requestContext, responseContext);
     verify(requestContext).getProperty("opencensus.context");
-    verify(responseContext, times(2)).getStatus();
+    verify(responseContext, times(1)).getStatus();
   }
 }
diff --git a/contrib/http_jetty_client/src/test/java/io/opencensus/contrib/http/jetty/client/HttpRequestListenerTest.java b/contrib/http_jetty_client/src/test/java/io/opencensus/contrib/http/jetty/client/HttpRequestListenerTest.java
@@ -23,7 +23,6 @@
 import io.opencensus.contrib.http.HttpClientHandler;
 import io.opencensus.contrib.http.HttpExtractor;
 import io.opencensus.contrib.http.HttpRequestContext;
-import io.opencensus.trace.Span;
 import io.opencensus.trace.Tracing;
 import io.opencensus.trace.propagation.TextFormat.Setter;
 import javax.annotation.Nullable;
@@ -101,22 +100,7 @@ public int getStatusCode(@Nullable Object response) {
           Tracing.getTracer(),
           extractor,
           Tracing.getPropagationComponent().getTraceContextFormat(),
-          setter) {
-        @Override
-        public HttpRequestContext handleStart(
-            @Nullable Span parent, Object carrier, Object request) {
-          return super.handleStart(parent, carrier, request);
-        }
-
-        @Override
-        public void handleEnd(
-            HttpRequestContext context,
-            @Nullable Object request,
-            @Nullable Object response,
-            @Nullable Throwable error) {
-          super.handleEnd(context, request, response, error);
-        }
-      };
+          setter) {};
   @Mock private HttpClientHandler<Request, Response, Request> mockHandler;
   @Mock private Result mockResult;
 
diff --git a/contrib/http_servlet/src/main/java/io/opencensus/contrib/http/servlet/OcHttpServletUtil.java b/contrib/http_servlet/src/main/java/io/opencensus/contrib/http/servlet/OcHttpServletUtil.java
@@ -92,7 +92,7 @@ static void recordMessageSentEvent(
    * @since 0.20.0
    */
   @MustBeClosed
-  public static Scope withScope(ServletRequest request) throws NullPointerException {
+  public static Scope withScope(ServletRequest request) {
     checkNotNull(request, "request");
     OcHttpServletListener listener =
         (OcHttpServletListener) request.getAttribute(OPENCENSUS_SERVLET_LISTENER);
diff --git a/contrib/http_servlet/src/test/java/io/opencensus/contrib/http/servlet/OcHttpServletListenerTest.java b/contrib/http_servlet/src/test/java/io/opencensus/contrib/http/servlet/OcHttpServletListenerTest.java
@@ -105,21 +105,7 @@ public int getStatusCode(@Nullable Object response) {
           extractor,
           Tracing.getPropagationComponent().getTraceContextFormat(),
           getter,
-          true) {
-        @Override
-        public HttpRequestContext handleStart(Object carrier, Object request) {
-          return super.handleStart(carrier, request);
-        }
-
-        @Override
-        public void handleEnd(
-            HttpRequestContext context,
-            Object request,
-            @Nullable Object response,
-            @Nullable Throwable error) {
-          super.handleEnd(context, request, response, error);
-        }
-      };
+          true) {};
   @Mock HttpRequestContext mockContext;
   @Mock HttpServletResponse mockResponse;
   @Mock HttpServletRequest mockRequest;
diff --git a/contrib/http_util/src/main/java/io/opencensus/contrib/http/AbstractHttpHandler.java b/contrib/http_util/src/main/java/io/opencensus/contrib/http/AbstractHttpHandler.java
@@ -101,14 +101,13 @@ public final void handleMessageReceived(HttpRequestContext context, long bytes)
     }
   }
 
-  void spanEnd(Span span, @Nullable P response, @Nullable Throwable error) {
-    int statusCode = extractor.getStatusCode(response);
+  void spanEnd(Span span, int httpStatus, @Nullable Throwable error) {
     if (span.getOptions().contains(Options.RECORD_EVENTS)) {
       span.putAttribute(
           HttpTraceAttributeConstants.HTTP_STATUS_CODE,
-          AttributeValue.longAttributeValue(statusCode));
+          AttributeValue.longAttributeValue(httpStatus));
+      span.setStatus(HttpTraceUtil.parseResponseStatus(httpStatus, error));
     }
-    span.setStatus(HttpTraceUtil.parseResponseStatus(statusCode, error));
     span.end();
   }
 
diff --git a/contrib/http_util/src/main/java/io/opencensus/contrib/http/HttpClientHandler.java b/contrib/http_util/src/main/java/io/opencensus/contrib/http/HttpClientHandler.java
@@ -147,19 +147,15 @@ public void handleEnd(
       @Nullable P response,
       @Nullable Throwable error) {
     checkNotNull(context, "context");
-    recordStats(context, request, response, error);
-    spanEnd(context.span, response, error);
+    int httpCode = extractor.getStatusCode(response);
+    recordStats(context, request, httpCode);
+    spanEnd(context.span, httpCode, error);
   }
 
-  private void recordStats(
-      HttpRequestContext context,
-      @Nullable Q request,
-      @Nullable P response,
-      @Nullable Throwable error) {
+  private void recordStats(HttpRequestContext context, @Nullable Q request, int httpCode) {
     double requestLatency = NANOSECONDS.toMillis(System.nanoTime() - context.requestStartTime);
 
     String methodStr = request == null ? "" : extractor.getMethod(request);
-    int status = extractor.getStatusCode(response);
     TagContext startCtx =
         tagger
             .toBuilder(context.tagContext)
@@ -169,7 +165,7 @@ private void recordStats(
                 METADATA_NO_PROPAGATION)
             .put(
                 HTTP_CLIENT_STATUS,
-                TagValue.create(status == 0 ? "error" : Integer.toString(status)),
+                TagValue.create(httpCode == 0 ? "error" : Integer.toString(httpCode)),
                 METADATA_NO_PROPAGATION)
             .build();
 
diff --git a/contrib/http_util/src/main/java/io/opencensus/contrib/http/HttpServerHandler.java b/contrib/http_util/src/main/java/io/opencensus/contrib/http/HttpServerHandler.java
@@ -163,17 +163,16 @@ public void handleEnd(
       HttpRequestContext context, Q request, @Nullable P response, @Nullable Throwable error) {
     checkNotNull(context, "context");
     checkNotNull(request, "request");
-    recordStats(context, request, response, error);
-    spanEnd(context.span, response, error);
+    int httpCode = extractor.getStatusCode(response);
+    recordStats(context, request, httpCode);
+    spanEnd(context.span, httpCode, error);
   }
 
-  private void recordStats(
-      HttpRequestContext context, Q request, @Nullable P response, @Nullable Throwable error) {
+  private void recordStats(HttpRequestContext context, Q request, int httpCode) {
     double requestLatency = NANOSECONDS.toMillis(System.nanoTime() - context.requestStartTime);
 
     String methodStr = extractor.getMethod(request);
     String routeStr = extractor.getRoute(request);
-    int status = extractor.getStatusCode(response);
     TagContext startCtx =
         tagger
             .toBuilder(context.tagContext)
@@ -187,7 +186,7 @@ private void recordStats(
                 METADATA_NO_PROPAGATION)
             .put(
                 HTTP_SERVER_STATUS,
-                TagValue.create(status == 0 ? "error" : Integer.toString(status)),
+                TagValue.create(httpCode == 0 ? "error" : Integer.toString(httpCode)),
                 METADATA_NO_PROPAGATION)
             .build();
 
diff --git a/contrib/http_util/src/test/java/io/opencensus/contrib/http/AbstractHttpHandlerTest.java b/contrib/http_util/src/test/java/io/opencensus/contrib/http/AbstractHttpHandlerTest.java
@@ -34,6 +34,7 @@
 import io.opencensus.trace.Span.Options;
 import io.opencensus.trace.SpanContext;
 import io.opencensus.trace.SpanId;
+import io.opencensus.trace.Status;
 import io.opencensus.trace.TraceId;
 import io.opencensus.trace.TraceOptions;
 import java.util.EnumSet;
@@ -59,7 +60,6 @@ public class AbstractHttpHandlerTest {
 
   @Rule public final ExpectedException thrown = ExpectedException.none();
   private final Object request = new Object();
-  private final Object response = new Object();
   private final Exception error = new Exception("test");
   private final Random random = new Random();
   private final SpanContext spanContext =
@@ -74,6 +74,7 @@ public class AbstractHttpHandlerTest {
   private AbstractHttpHandler<Object, Object> handler;
   @Captor private ArgumentCaptor<MessageEvent> captor;
   @Captor private ArgumentCaptor<AttributeValue> attributeCaptor;
+  @Captor private ArgumentCaptor<Status> statusCaptor;
   @Captor private ArgumentCaptor<EndSpanOptions> optionsCaptor;
 
   @Spy private FakeSpan fakeSpan = new FakeSpan(spanContext, EnumSet.of(Options.RECORD_EVENTS));
@@ -130,32 +131,39 @@ public void handleMessageReceived() {
   @Test
   public void handleEndDisallowNullSpan() {
     thrown.expect(NullPointerException.class);
-    handler.spanEnd(null, response, error);
+    handler.spanEnd(null, 0, error);
   }
 
   @Test
-  public void handleEndAllowNullResponseAndError() {
-    handler.spanEnd(fakeSpan, /*response=*/ null, /*error=*/ null);
+  public void handleEndAllowZeroCodeAndNullError() {
+    handler.spanEnd(fakeSpan, 0, null);
+    verify(fakeSpan).setStatus(statusCaptor.capture());
+    assertThat(statusCaptor.getValue()).isEqualTo(Status.UNKNOWN);
+  }
+
+  @Test
+  public void handleEndAllowNonZeroCodeAndNullError() {
+    handler.spanEnd(fakeSpan, 200, null);
+    verify(fakeSpan).setStatus(statusCaptor.capture());
+    assertThat(statusCaptor.getValue()).isEqualTo(Status.OK);
   }
 
   @Test
   public void handleEndShouldEndSpan() {
     when(extractor.getStatusCode(any(Object.class))).thenReturn(0);
 
-    handler.spanEnd(fakeSpan, response, error);
+    handler.spanEnd(fakeSpan, 0, error);
     verify(fakeSpan).end(optionsCaptor.capture());
-    EndSpanOptions options = optionsCaptor.getValue();
-    assertThat(options).isEqualTo(EndSpanOptions.DEFAULT);
+    assertThat(optionsCaptor.getValue()).isEqualTo(EndSpanOptions.DEFAULT);
   }
 
   @Test
   public void handleEndWithRecordEvents() {
     when(extractor.getStatusCode(any(Object.class))).thenReturn(0);
-    handler.spanEnd(fakeSpan, response, error);
+    handler.spanEnd(fakeSpan, 0, error);
     verify(fakeSpan)
         .putAttribute(eq(HttpTraceAttributeConstants.HTTP_STATUS_CODE), attributeCaptor.capture());
-    AttributeValue attribute = attributeCaptor.getValue();
-    assertThat(attribute).isEqualTo(AttributeValue.longAttributeValue(0));
+    assertThat(attributeCaptor.getValue()).isEqualTo(AttributeValue.longAttributeValue(0));
   }
 
   @Test
@@ -166,8 +174,7 @@ public void testSpanName() {
 
   private void verifyAttributes(String key) {
     verify(span).putAttribute(eq(key), attributeCaptor.capture());
-    AttributeValue attribute = attributeCaptor.getValue();
-    assertThat(attribute.toString()).contains(attributeMap.get(key));
+    assertThat(attributeCaptor.getValue().toString()).contains(attributeMap.get(key));
   }
 
   @Test
diff --git a/contrib/zpages/src/main/java/io/opencensus/contrib/zpages/ZPageHttpHandler.java b/contrib/zpages/src/main/java/io/opencensus/contrib/zpages/ZPageHttpHandler.java
@@ -83,6 +83,6 @@ static Map<String, String> uriQueryToMap(URI uri) {
         result.put(splits.get(0), "");
       }
     }
-    return result;
+    return Collections.unmodifiableMap(result);
   }
 }
diff --git a/contrib/zpages/src/test/java/io/opencensus/contrib/zpages/RpczZPageHandlerTest.java b/contrib/zpages/src/test/java/io/opencensus/contrib/zpages/RpczZPageHandlerTest.java
@@ -38,14 +38,13 @@
 import org.junit.Test;
 import org.junit.runner.RunWith;
 import org.junit.runners.JUnit4;
-import org.mockito.Mock;
 import org.mockito.Mockito;
 
 /** Unit tests for {@link RpczZPageHandler}. */
 @RunWith(JUnit4.class)
 public class RpczZPageHandlerTest {
 
-  @Mock private final ViewManager mockViewManager = Mockito.mock(ViewManager.class);
+  private final ViewManager mockViewManager = Mockito.mock(ViewManager.class);
 
   private static final TagValue METHOD_1 = TagValue.create("method1");
   private static final TagValue METHOD_2 = TagValue.create("method2");
diff --git a/contrib/zpages/src/test/java/io/opencensus/contrib/zpages/StatszZPageHandlerTest.java b/contrib/zpages/src/test/java/io/opencensus/contrib/zpages/StatszZPageHandlerTest.java
@@ -54,14 +54,13 @@
 import org.junit.Test;
 import org.junit.runner.RunWith;
 import org.junit.runners.JUnit4;
-import org.mockito.Mock;
 import org.mockito.Mockito;
 
 /** Unit tests for {@link StatszZPageHandler}. */
 @RunWith(JUnit4.class)
 public class StatszZPageHandlerTest {
 
-  @Mock private final ViewManager mockViewManager = Mockito.mock(ViewManager.class);
+  private final ViewManager mockViewManager = Mockito.mock(ViewManager.class);
 
   private static final View MY_VIEW =
       View.create(
diff --git a/exporters/trace/ocagent/src/main/java/io/opencensus/exporter/trace/ocagent/OcAgentTraceExporterHandler.java b/exporters/trace/ocagent/src/main/java/io/opencensus/exporter/trace/ocagent/OcAgentTraceExporterHandler.java
@@ -40,7 +40,7 @@ final class OcAgentTraceExporterHandler extends Handler {
 
   @VisibleForTesting static final String DEFAULT_END_POINT = "localhost:55678";
   private static final String DEFAULT_SERVICE_NAME = "OpenCensus";
-  private static final Duration DEFAULT_RETRY_INTERVAL = Duration.create(300, 0); // 5 minutes
+  // private static final Duration DEFAULT_RETRY_INTERVAL = Duration.create(300, 0); // 5 minutes
 
   private final String endPoint;
   private final Node node;
diff --git a/impl_core/src/main/java/io/opencensus/implcore/metrics/MetricRegistryImpl.java b/impl_core/src/main/java/io/opencensus/implcore/metrics/MetricRegistryImpl.java
@@ -146,7 +146,7 @@ public Collection<Metric> getMetrics() {
           metrics.add(metric);
         }
       }
-      return metrics;
+      return Collections.unmodifiableCollection(metrics);
     }
   }
 
diff --git a/impl_core/src/test/java/io/opencensus/implcore/metrics/MetricRegistryImplTest.java b/impl_core/src/test/java/io/opencensus/implcore/metrics/MetricRegistryImplTest.java

Original file line number	Diff line number	Diff line change
`@@ -67,7 +67,7 @@ public void testResponseFilter() throws Exception {`
`67`	`67`	`filter.filter(requestContext, responseContext);`
`68`	`68`
`69`	`69`	`verify(requestContext).getProperty("opencensus.context");`
`70`		`- verify(responseContext, times(2)).getStatus();`
	`70`	`+ verify(responseContext, times(1)).getStatus();`
`71`	`71`	`}`
`72`	`72`
`73`	`73`	`static HttpRequestContext createHttpRequestContext(Span span, TagContext tagContext)`
Original file line number	Diff line number	Diff line change
`@@ -72,6 +72,6 @@ public void testResponseFilter() throws Exception {`
`72`	`72`	`ContainerResponseContext responseContext = mock(ContainerResponseContext.class);`
`73`	`73`	`filter.filter(requestContext, responseContext);`
`74`	`74`	`verify(requestContext).getProperty("opencensus.context");`
`75`		`- verify(responseContext, times(2)).getStatus();`
	`75`	`+ verify(responseContext, times(1)).getStatus();`
`76`	`76`	`}`
`77`	`77`	`}`
Original file line number	Diff line number	Diff line change
`@@ -101,14 +101,13 @@ public final void handleMessageReceived(HttpRequestContext context, long bytes)`
`101`	`101`	`}`
`102`	`102`	`}`
`103`	`103`
`104`		`- void spanEnd(Span span, @Nullable P response, @Nullable Throwable error) {`
`105`		`- int statusCode = extractor.getStatusCode(response);`
	`104`	`+ void spanEnd(Span span, int httpStatus, @Nullable Throwable error) {`
`106`	`105`	`if (span.getOptions().contains(Options.RECORD_EVENTS)) {`
`107`	`106`	`span.putAttribute(`
`108`	`107`	`HttpTraceAttributeConstants.HTTP_STATUS_CODE,`
`109`		`- AttributeValue.longAttributeValue(statusCode));`
	`108`	`+ AttributeValue.longAttributeValue(httpStatus));`
	`109`	`+ span.setStatus(HttpTraceUtil.parseResponseStatus(httpStatus, error));`
`110`	`110`	`}`
`111`		`- span.setStatus(HttpTraceUtil.parseResponseStatus(statusCode, error));`
`112`	`111`	`span.end();`
`113`	`112`	`}`
`114`	`113`