Only upload stats for valid views. Improve logging. (#841)

(cherry picked from commit f502b66)

Author: songy23

exporters/stats/stackdriver/src/main/java/io/opencensus/exporter/stats/stackdriver/StackdriverExportUtils.java

@@ -139,6 +139,9 @@ static MetricDescriptor.ValueType createValueType(
   // Convert ViewData to a list of TimeSeries, so that ViewData can be uploaded to Stackdriver.
   static List<TimeSeries> createTimeSeriesList(ViewData viewData, String projectId) {
     List<TimeSeries> timeSeriesList = Lists.newArrayList();
+    if (viewData == null) {
+      return timeSeriesList;
+    }
     View view = viewData.getView();
     if (!(view.getWindow() instanceof Cumulative)) {
       // TODO(songya): Only Cumulative view will be exported to Stackdriver in this version.

exporters/stats/stackdriver/src/main/java/io/opencensus/exporter/stats/stackdriver/StackdriverExporterWorkerThread.java

@@ -72,23 +72,22 @@ final class StackdriverExporterWorkerThread extends Thread {
     setName("ExportWorkerThread");
   }
 
-  // Registers the given view to Stackdriver Monitoring if the view is not registered yet, otherwise
-  // does nothing. Throws IllegalArgumentException if the given view is a different view that has a
-  // same name with a registered view.
+  // Returns true if the given view is successfully registered to Stackdriver Monitoring, or the
+  // exact same view has already been registered. Returns false otherwise.
   @VisibleForTesting
-  void registerView(View view) {
+  boolean registerView(View view) {
     View existing = registeredViews.get(view.getName());
     if (existing != null) {
       if (existing.equals(view)) {
         // Ignore views that are already registered.
-        return;
+        return true;
       } else {
         // If we upload a view that has the same name with a registered view but with different
         // attributes, Stackdriver client will throw an exception.
         logger.log(
             Level.WARNING,
             "A different view with the same name is already registered: " + existing);
-        return;
+        return false;
       }
     }
     registeredViews.put(view.getName(), view);
@@ -98,12 +97,22 @@ void registerView(View view) {
     // views should be pre-registered.
     MetricDescriptor metricDescriptor =
         StackdriverExportUtils.createMetricDescriptor(view, projectId);
-    if (metricDescriptor != null) {
-      metricServiceClient.createMetricDescriptor(
-          CreateMetricDescriptorRequest.newBuilder()
-              .setNameWithProjectName(projectName)
-              .setMetricDescriptor(metricDescriptor)
-              .build());
+    if (metricDescriptor == null) {
+      // Don't register interval views in this version.
+      return false;
+    }
+
+    CreateMetricDescriptorRequest request =
+        CreateMetricDescriptorRequest.newBuilder()
+            .setNameWithProjectName(projectName)
+            .setMetricDescriptor(metricDescriptor)
+            .build();
+    try {
+      metricServiceClient.createMetricDescriptor(request);
+      return true;
+    } catch (Throwable e) {
+      logger.log(Level.WARNING, "Exception thrown when registering MetricDescriptor.", e);
+      return false;
     }
   }
 
@@ -113,8 +122,10 @@ void registerView(View view) {
   void export() {
     List<ViewData> viewDataList = Lists.newArrayList();
     for (View view : viewManager.getAllExportedViews()) {
-      registerView(view);
-      viewDataList.add(viewManager.getView(view.getName()));
+      if (registerView(view)) {
+        // Only upload stats for valid views.
+        viewDataList.add(viewManager.getView(view.getName()));
+      }
     }
     List<TimeSeries> timeSeriesList = Lists.newArrayList();
     for (ViewData viewData : viewDataList) {
@@ -124,13 +135,13 @@ void export() {
     for (List<TimeSeries> batchedTimeSeries :
         Lists.partition(timeSeriesList, MAX_BATCH_EXPORT_SIZE)) {
       // Batch export 3 TimeSeries at one call, to avoid exceeding RPC header size limit.
-      CreateTimeSeriesRequest.Builder builder =
-          CreateTimeSeriesRequest.newBuilder().setNameWithProjectName(projectName);
-      for (TimeSeries timeSeries : batchedTimeSeries) {
-        builder.addTimeSeries(timeSeries);
-      }
+      CreateTimeSeriesRequest request =
+          CreateTimeSeriesRequest.newBuilder()
+              .setNameWithProjectName(projectName)
+              .addAllTimeSeries(batchedTimeSeries)
+              .build();
       try {
-        metricServiceClient.createTimeSeries(builder.build());
+        metricServiceClient.createTimeSeries(request);
       } catch (Throwable e) {
         logger.log(Level.WARNING, "Exception thrown when exporting TimeSeries.", e);
       }

exporters/stats/stackdriver/src/test/java/io/opencensus/exporter/stats/stackdriver/StackdriverExporterWorkerThreadTest.java

@@ -20,6 +20,7 @@
 import static org.mockito.Matchers.any;
 import static org.mockito.Matchers.eq;
 import static org.mockito.Mockito.doReturn;
+import static org.mockito.Mockito.doThrow;
 import static org.mockito.Mockito.times;
 import static org.mockito.Mockito.verify;
 
@@ -168,14 +169,30 @@ public void doNotExportForEmptyViewData() {
     verify(mockStub, times(0)).createTimeSeriesCallable();
   }
 
+  @Test
+  public void doNotExportIfFailedToRegisterView() {
+    View view =
+        View.create(VIEW_NAME, VIEW_DESCRIPTION, MEASURE, SUM, Arrays.asList(KEY), CUMULATIVE);
+    doReturn(ImmutableSet.of(view)).when(mockViewManager).getAllExportedViews();
+    doThrow(new IllegalArgumentException()).when(mockStub).createMetricDescriptorCallable();
+    StackdriverExporterWorkerThread workerThread =
+        new StackdriverExporterWorkerThread(
+            PROJECT_ID, new FakeMetricServiceClient(mockStub), ONE_SECOND, mockViewManager);
+
+    assertThat(workerThread.registerView(view)).isFalse();
+    workerThread.export();
+    verify(mockStub, times(1)).createMetricDescriptorCallable();
+    verify(mockStub, times(0)).createTimeSeriesCallable();
+  }
+
   @Test
   public void skipDifferentViewWithSameName() throws IOException {
     StackdriverExporterWorkerThread workerThread =
         new StackdriverExporterWorkerThread(
             PROJECT_ID, new FakeMetricServiceClient(mockStub), ONE_SECOND, mockViewManager);
     View view1 =
         View.create(VIEW_NAME, VIEW_DESCRIPTION, MEASURE, SUM, Arrays.asList(KEY), CUMULATIVE);
-    workerThread.registerView(view1);
+    assertThat(workerThread.registerView(view1)).isTrue();
     verify(mockStub, times(1)).createMetricDescriptorCallable();
 
     View view2 =
@@ -186,7 +203,7 @@ public void skipDifferentViewWithSameName() throws IOException {
             SUM,
             Arrays.asList(KEY),
             CUMULATIVE);
-    workerThread.registerView(view2);
+    assertThat(workerThread.registerView(view2)).isFalse();
     verify(mockStub, times(1)).createMetricDescriptorCallable();
   }
 
@@ -197,10 +214,10 @@ public void doNotCreateMetricDescriptorForRegisteredView() {
             PROJECT_ID, new FakeMetricServiceClient(mockStub), ONE_SECOND, mockViewManager);
     View view =
         View.create(VIEW_NAME, VIEW_DESCRIPTION, MEASURE, SUM, Arrays.asList(KEY), CUMULATIVE);
-    workerThread.registerView(view);
+    assertThat(workerThread.registerView(view)).isTrue();
     verify(mockStub, times(1)).createMetricDescriptorCallable();
 
-    workerThread.registerView(view);
+    assertThat(workerThread.registerView(view)).isTrue();
     verify(mockStub, times(1)).createMetricDescriptorCallable();
   }
 
@@ -211,7 +228,7 @@ public void doNotCreateMetricDescriptorForIntervalView() {
             PROJECT_ID, new FakeMetricServiceClient(mockStub), ONE_SECOND, mockViewManager);
     View view =
         View.create(VIEW_NAME, VIEW_DESCRIPTION, MEASURE, SUM, Arrays.asList(KEY), INTERVAL);
-    workerThread.registerView(view);
+    assertThat(workerThread.registerView(view)).isFalse();
     verify(mockStub, times(0)).createMetricDescriptorCallable();
   }