Changes to the stats exporter interface (#59)

* Add an interface for pull exporters.

The present exporter interface is unsuited for pull exporters--they need to store data from timed exports, and have no robust means of telling when a view has been unregistered and should no longer be exporting. This adds a direct interface to retrieve fresh data from registered views.

Since accessing the ViewData only requires read-only access to the registered views, exporters using this pull interface will not contend with the timed push exports.

* Disallow registering non-cumulative views with the stats exporter.

Author: Ian Sturdy

opencensus/exporters/stats/stackdriver/internal/stackdriver_exporter.cc

@@ -73,11 +73,6 @@ void StackdriverExporter::Handler::ExportViewData(
     const opencensus::stats::ViewDescriptor& descriptor,
     const opencensus::stats::ViewData& data) {
   absl::MutexLock l(&mu_);
-  // Stackdriver does not support interval aggregation.
-  if (descriptor.aggregation_window().type() !=
-      opencensus::stats::AggregationWindow::Type::kCumulative) {
-    return;
-  }
   if (!MaybeRegisterView(descriptor)) {
     return;
   }
@@ -134,7 +129,7 @@ bool StackdriverExporter::Handler::MaybeRegisterView(
 // static
 void StackdriverExporter::Register(absl::string_view project_id,
                                    absl::string_view opencensus_task) {
-  opencensus::stats::StatsExporter::RegisterHandler(absl::WrapUnique(
+  opencensus::stats::StatsExporter::RegisterPushHandler(absl::WrapUnique(
       new StackdriverExporter::Handler(project_id, opencensus_task)));
 }
 

opencensus/exporters/stats/stdout/internal/stdout_exporter.cc

@@ -55,7 +55,7 @@ class StdoutExporter::Handler
 
 // static
 void StdoutExporter::Register() {
-  opencensus::stats::StatsExporter::RegisterHandler(
+  opencensus::stats::StatsExporter::RegisterPushHandler(
       absl::make_unique<StdoutExporter::Handler>());
 }
 

opencensus/stats/BUILD

@@ -183,6 +183,7 @@ cc_test(
     deps = [
         ":core",
         ":export",
+        ":recording",
         "@com_google_absl//absl/memory",
         "@com_google_absl//absl/time",
         "@com_google_googletest//:gtest_main",

opencensus/stats/examples/exporter_example.cc

@@ -38,7 +38,7 @@ opencensus::stats::MeasureDouble FooUsage() {
 class ExampleExporter : public opencensus::stats::StatsExporter::Handler {
  public:
   static void Register() {
-    opencensus::stats::StatsExporter::RegisterHandler(
+    opencensus::stats::StatsExporter::RegisterPushHandler(
         absl::make_unique<ExampleExporter>());
   }
 

opencensus/stats/internal/stats_exporter.cc

@@ -14,12 +14,16 @@
 
 #include "opencensus/stats/stats_exporter.h"
 
+#include <iostream>
 #include <thread>  // NOLINT
+#include <utility>
+#include <vector>
 
 #include "absl/memory/memory.h"
 #include "absl/synchronization/mutex.h"
 #include "absl/time/clock.h"
 #include "absl/time/time.h"
+#include "opencensus/stats/internal/aggregation_window.h"
 
 namespace opencensus {
 namespace stats {
@@ -45,16 +49,26 @@ class StatsExporterImpl {
   // Adds a handler, which cannot be subsequently removed (except by
   // ClearHandlersForTesting()). The background thread is started when the
   // first handler is registered.
-  void RegisterHandler(std::unique_ptr<StatsExporter::Handler> handler) {
+  void RegisterPushHandler(std::unique_ptr<StatsExporter::Handler> handler) {
     absl::MutexLock l(&mu_);
     handlers_.push_back(std::move(handler));
     if (!thread_started_) {
       StartExportThread();
     }
   }
 
+  std::vector<std::pair<ViewDescriptor, ViewData>> GetViewData() {
+    absl::ReaderMutexLock l(&mu_);
+    std::vector<std::pair<ViewDescriptor, ViewData>> data;
+    data.reserve(views_.size());
+    for (const auto& view : views_) {
+      data.emplace_back(view.second->descriptor(), view.second->GetData());
+    }
+    return data;
+  }
+
   void Export() {
-    absl::MutexLock l(&mu_);
+    absl::ReaderMutexLock l(&mu_);
     for (const auto& view : views_) {
       SendToHandlers(view.second->descriptor(), view.second->GetData());
     }
@@ -106,15 +120,24 @@ class StatsExporterImpl {
 };
 
 void StatsExporter::AddView(const ViewDescriptor& view) {
-  StatsExporterImpl::Get()->AddView(view);
+  if (view.aggregation_window().type() ==
+      AggregationWindow::Type::kCumulative) {
+    StatsExporterImpl::Get()->AddView(view);
+  } else {
+    std::cerr << "Only cumulative views may be registered for export.\n";
+  }
 }
 
 void StatsExporter::RemoveView(absl::string_view name) {
   StatsExporterImpl::Get()->RemoveView(name);
 }
 
-void StatsExporter::RegisterHandler(std::unique_ptr<Handler> handler) {
-  StatsExporterImpl::Get()->RegisterHandler(std::move(handler));
+void StatsExporter::RegisterPushHandler(std::unique_ptr<Handler> handler) {
+  StatsExporterImpl::Get()->RegisterPushHandler(std::move(handler));
+}
+
+std::vector<std::pair<ViewDescriptor, ViewData>> StatsExporter::GetViewData() {
+  return StatsExporterImpl::Get()->GetViewData();
 }
 
 void StatsExporter::ExportForTesting() { StatsExporterImpl::Get()->Export(); }

opencensus/stats/internal/stats_exporter_test.cc

@@ -15,13 +15,15 @@
 #include "opencensus/stats/stats_exporter.h"
 
 #include <cstdint>
+#include <utility>
 #include <vector>
 
 #include "absl/memory/memory.h"
 #include "absl/time/clock.h"
 #include "absl/time/time.h"
 #include "gmock/gmock.h"
 #include "gtest/gtest.h"
+#include "opencensus/stats/internal/set_aggregation_window.h"
 #include "opencensus/stats/measure.h"
 #include "opencensus/stats/measure_descriptor.h"
 #include "opencensus/stats/measure_registry.h"
@@ -35,7 +37,7 @@ namespace stats {
 class MockExporter : public StatsExporter::Handler {
  public:
   static void Register(absl::Span<const ViewDescriptor> expected_descriptors) {
-    opencensus::stats::StatsExporter::RegisterHandler(
+    opencensus::stats::StatsExporter::RegisterPushHandler(
         absl::make_unique<MockExporter>(expected_descriptors));
   }
 
@@ -81,8 +83,6 @@ class StatsExporterTest : public ::testing::Test {
     descriptor2_.set_measure(kMeasureId);
     descriptor2_.set_aggregation(
         Aggregation::Distribution(BucketBoundaries::Explicit({0})));
-    SetAggregationWindow(AggregationWindow::Interval(absl::Hours(1)),
-                         &descriptor2_);
   }
 
   void TearDown() {
@@ -102,6 +102,9 @@ TEST_F(StatsExporterTest, AddView) {
   MockExporter::Register({descriptor1_, descriptor2_});
   StatsExporter::AddView(descriptor1_);
   StatsExporter::AddView(descriptor2_);
+  EXPECT_THAT(StatsExporter::GetViewData(),
+              ::testing::UnorderedElementsAre(::testing::Key(descriptor1_),
+                                              ::testing::Key(descriptor2_)));
   Export();
 }
 
@@ -110,6 +113,10 @@ TEST_F(StatsExporterTest, UpdateView) {
   StatsExporter::AddView(descriptor1_);
   StatsExporter::AddView(descriptor2_);
   StatsExporter::AddView(descriptor1_edited_);
+  EXPECT_THAT(
+      StatsExporter::GetViewData(),
+      ::testing::UnorderedElementsAre(::testing::Key(descriptor1_edited_),
+                                      ::testing::Key(descriptor2_)));
   Export();
 }
 
@@ -118,6 +125,8 @@ TEST_F(StatsExporterTest, RemoveView) {
   StatsExporter::AddView(descriptor1_);
   StatsExporter::AddView(descriptor2_);
   StatsExporter::RemoveView(descriptor1_.name());
+  EXPECT_THAT(StatsExporter::GetViewData(),
+              ::testing::UnorderedElementsAre(::testing::Key(descriptor2_)));
   Export();
 }
 
@@ -128,6 +137,16 @@ TEST_F(StatsExporterTest, MultipleExporters) {
   Export();
 }
 
+TEST_F(StatsExporterTest, IntervalViewRejected) {
+  MockExporter::Register({});
+  ViewDescriptor interval_descriptor = ViewDescriptor().set_name("interval");
+  SetAggregationWindow(AggregationWindow::Interval(absl::Hours(1)),
+                       &interval_descriptor);
+  StatsExporter::AddView(interval_descriptor);
+  EXPECT_TRUE(StatsExporter::GetViewData().empty());
+  Export();
+}
+
 TEST_F(StatsExporterTest, TimedExport) {
   MockExporter::Register({descriptor1_});
   StatsExporter::AddView(descriptor1_);

opencensus/stats/stats_exporter.h

@@ -19,6 +19,8 @@
 #include <functional>
 #include <memory>
 #include <unordered_map>
+#include <utility>
+#include <vector>
 
 #include "absl/strings/string_view.h"
 #include "opencensus/stats/view.h"
@@ -32,24 +34,29 @@ namespace stats {
 // StatsExporter is thread-safe.
 class StatsExporter final {
  public:
-  // Inserts a new view, replacing any existing view with the same name.
+  // Inserts a new view, replacing any existing view with the same name. Only
+  // Cumulative views are supported.
   static void AddView(const ViewDescriptor& view);
   // Removes the view with 'name' from the registry, if one is registered.
   static void RemoveView(absl::string_view name);
 
-  // StatsExporter::Handler is the interface for exporters that export recorded
-  // data for registered views. The exporter should provide a static Register()
-  // method that takes any arguments needed by the exporter (e.g. a URL to
-  // export to) and calls StatsExporter::RegisterHandler itself.
+  // StatsExporter::Handler is the interface for push exporters that export
+  // recorded data for registered views. The exporter should provide a static
+  // Register() method that takes any arguments needed by the exporter (e.g. a
+  // URL to export to) and calls StatsExporter::RegisterHandler itself.
   class Handler {
    public:
     virtual ~Handler() = default;
     virtual void ExportViewData(const ViewDescriptor& descriptor,
                                 const ViewData& data) = 0;
   };
 
-  // This should only be called by Handler's Register() methods.
-  static void RegisterHandler(std::unique_ptr<Handler> handler);
+  // This should only be called by push exporters' Register() methods.
+  static void RegisterPushHandler(std::unique_ptr<Handler> handler);
+
+  // Retrieves current data for all registered views, for implementing pull
+  // exporters.
+  static std::vector<std::pair<ViewDescriptor, ViewData>> GetViewData();
 
  private:
   friend class StatsExporterTest;