From 0ca6905226363e87c4182fe4017af658e640339a Mon Sep 17 00:00:00 2001
From: Janet Vu <vjanet@umich.edu>
Date: Thu, 20 Aug 2020 16:04:41 +0000
Subject: [PATCH 01/40] Add initial benchmark code

---
 ext/test/zpages/BUILD                         | 20 +++++-
 ext/test/zpages/tracez_processor_benchmark.cc | 70 +++++++++++++++++++
 2 files changed, 87 insertions(+), 3 deletions(-)
 create mode 100644 ext/test/zpages/tracez_processor_benchmark.cc

diff --git a/ext/test/zpages/BUILD b/ext/test/zpages/BUILD
index 07c287d2fa..b7177b2d57 100644
--- a/ext/test/zpages/BUILD
+++ b/ext/test/zpages/BUILD
@@ -1,5 +1,7 @@
+load("//bazel:otel_cc_benchmark.bzl", "otel_cc_benchmark")
+
 cc_test(
-    name = "threadsafe_span_data_tests",
+    name = "threadsafe_span_data_test",
     srcs = [
         "threadsafe_span_data_test.cc",
     ],
@@ -11,7 +13,7 @@ cc_test(
 )
 
 cc_test(
-    name = "tracez_data_aggregator_tests",
+    name = "tracez_data_aggregator_test",
     srcs = [
         "tracez_data_aggregator_test.cc",
     ],
@@ -23,7 +25,7 @@ cc_test(
 )
 
 cc_test(
-    name = "tracez_processor_tests",
+    name = "tracez_processor_test",
     srcs = [
         "tracez_processor_test.cc",
     ],
@@ -33,3 +35,15 @@ cc_test(
         "@com_google_googletest//:gtest_main",
     ],
 )
+
+otel_cc_benchmark(
+    name = "tracez_processor_benchmark",
+    srcs = [
+        "tracez_processor_benchmark.cc",
+    ],
+    deps = [
+        "//ext/src/zpages",
+        "//sdk/src/trace",
+        "//api",
+    ],
+)
diff --git a/ext/test/zpages/tracez_processor_benchmark.cc b/ext/test/zpages/tracez_processor_benchmark.cc
new file mode 100644
index 0000000000..25c9da849f
--- /dev/null
+++ b/ext/test/zpages/tracez_processor_benchmark.cc
@@ -0,0 +1,70 @@
+#include "opentelemetry/ext/zpages/tracez_processor.h"
+
+#include <benchmark/benchmark.h>
+
+#include "opentelemetry/sdk/trace/tracer.h"
+#include "opentelemetry/nostd/shared_ptr.h"
+
+using namespace opentelemetry::sdk::trace;
+using namespace opentelemetry::ext::zpages;
+
+//////////////////////////////////// TEST HELPER FUNCTIONS //////////////////////////////
+
+/*
+ * Helper function that creates i spans, added into the passed in vector
+ */
+void StartManySameSpans(
+    std::vector<opentelemetry::nostd::shared_ptr<opentelemetry::trace::Span>> &spans,
+    std::shared_ptr<opentelemetry::trace::Tracer> tracer,
+    int i)
+{
+  for (; i > 0; i--)
+    spans.push_back(tracer->StartSpan("span"));
+}
+
+/*
+ * Helper function that ends all spans in the passed in span vector.
+ */
+void EndSpans(std::vector<opentelemetry::nostd::shared_ptr<opentelemetry::trace::Span>> &spans)
+{
+  for (auto &span : spans)
+    span->End();
+}
+
+//////////////////////////////// TEST FIXTURE //////////////////////////////////////
+
+/*
+ * Reduce code duplication by having single area with shared setup code
+ */
+class TracezProcessorBM : public benchmark::Fixture
+{
+protected:
+  void SetUp(const ::benchmark::State& state)
+  {
+    processor  = std::shared_ptr<TracezSpanProcessor>(new TracezSpanProcessor());
+    tracer     = std::shared_ptr<opentelemetry::trace::Tracer>(new Tracer(processor));
+
+  }
+
+  std::shared_ptr<TracezSpanProcessor> processor;
+  std::shared_ptr<opentelemetry::trace::Tracer> tracer;
+
+  std::vector<opentelemetry::nostd::shared_ptr<opentelemetry::trace::Span>> spans;
+
+  const int numIterations = 1000;
+  const int numSpans      = 500;
+};
+
+///////////////////////////////////////// TESTS ///////////////////////////////////
+
+BENCHMARK_F(TracezProcessorBM, BM_TracezProcessorManySameName)(benchmark::State &state)
+{
+  while(state.KeepRunningBatch(numIterations))
+  {
+    StartManySameSpans(spans, tracer, 500);
+    EndSpans(spans);
+  }
+}
+
+
+BENCHMARK_MAIN();

From 7a5960051ee04b4c10e0975ee61bf4bc0359a0b1 Mon Sep 17 00:00:00 2001
From: Janet Vu <vjanet@umich.edu>
Date: Fri, 21 Aug 2020 14:54:30 +0000
Subject: [PATCH 02/40] Add working tracez processor benchmark

---
 ext/test/zpages/tracez_processor_benchmark.cc | 111 +++++++++++++++---
 1 file changed, 93 insertions(+), 18 deletions(-)

diff --git a/ext/test/zpages/tracez_processor_benchmark.cc b/ext/test/zpages/tracez_processor_benchmark.cc
index 25c9da849f..5aaac7656c 100644
--- a/ext/test/zpages/tracez_processor_benchmark.cc
+++ b/ext/test/zpages/tracez_processor_benchmark.cc
@@ -1,49 +1,56 @@
+#include "opentelemetry/context/threadlocal_context.h"
 #include "opentelemetry/ext/zpages/tracez_processor.h"
 
 #include <benchmark/benchmark.h>
+#include <thread>
 
 #include "opentelemetry/sdk/trace/tracer.h"
-#include "opentelemetry/nostd/shared_ptr.h"
 
 using namespace opentelemetry::sdk::trace;
 using namespace opentelemetry::ext::zpages;
 
-//////////////////////////////////// TEST HELPER FUNCTIONS //////////////////////////////
+/////////////////////////////// BENCHMARK HELPER FUNCTIONS //////////////////////////////
 
 /*
- * Helper function that creates i spans, added into the passed in vector
+ * Helper function that creates i spans, which are added into the passed
+ * in vector. Used for testing thread safety
  */
-void StartManySameSpans(
+void StartManySpans(
     std::vector<opentelemetry::nostd::shared_ptr<opentelemetry::trace::Span>> &spans,
     std::shared_ptr<opentelemetry::trace::Tracer> tracer,
     int i)
 {
   for (; i > 0; i--)
-    spans.push_back(tracer->StartSpan("span"));
+    spans.push_back(tracer->StartSpan(""));
 }
 
 /*
  * Helper function that ends all spans in the passed in span vector.
  */
-void EndSpans(std::vector<opentelemetry::nostd::shared_ptr<opentelemetry::trace::Span>> &spans)
+void EndAllSpans(std::vector<opentelemetry::nostd::shared_ptr<opentelemetry::trace::Span>> &spans)
 {
   for (auto &span : spans)
     span->End();
 }
 
-//////////////////////////////// TEST FIXTURE //////////////////////////////////////
-
 /*
- * Reduce code duplication by having single area with shared setup code
+ * Helper function calls GetSpanSnapshot() i times, does nothing otherwise
  */
-class TracezProcessorBM : public benchmark::Fixture
+void GetManySnapshots(std::shared_ptr<TracezSpanProcessor> &processor, int i)
+{
+  for (; i > 0; i--)
+    processor->GetSpanSnapshot();
+}
+
+////////////////////////  FIXTURE FOR SHARED SETUP CODE ///////////////////
+
+class TracezProcessor : public benchmark::Fixture
 {
 protected:
   void SetUp(const ::benchmark::State& state)
   {
     processor  = std::shared_ptr<TracezSpanProcessor>(new TracezSpanProcessor());
     tracer     = std::shared_ptr<opentelemetry::trace::Tracer>(new Tracer(processor));
-
   }
 
   std::shared_ptr<TracezSpanProcessor> processor;
@@ -51,20 +58,88 @@ class TracezProcessorBM : public benchmark::Fixture
 
   std::vector<opentelemetry::nostd::shared_ptr<opentelemetry::trace::Span>> spans;
 
-  const int numIterations = 1000;
-  const int numSpans      = 500;
+  const int numSpans      = 50;
 };
 
-///////////////////////////////////////// TESTS ///////////////////////////////////
+//////////////////////////// BENCHMARK DEFINITIONS /////////////////////////////////
+
+/*
+ * Make and end many empty spans.
+ */
+
+BENCHMARK_DEFINE_F(TracezProcessor, BM_RunComplete)(benchmark::State &state)
+{
+  for (auto _ : state)
+  {
+    StartManySpans(spans, tracer, numSpans);
+    EndAllSpans(spans);
+  }
+}
+
+/*
+ * Make many empty spans while spapshots grabbed.
+ */
+BENCHMARK_DEFINE_F(TracezProcessor, BM_RunSnap)(benchmark::State &state)
+{
+  for (auto _ : state)
+  {
+    std::thread start(StartManySpans, std::ref(spans), tracer, numSpans);
+    std::thread snapshots(GetManySnapshots, std::ref(processor), numSpans);
+
+    start.join();
+    snapshots.join();
 
-BENCHMARK_F(TracezProcessorBM, BM_TracezProcessorManySameName)(benchmark::State &state)
+    EndAllSpans(spans);
+  }
+}
+
+/*
+ * Make many empty spans end while snapshots are being grabbed.
+ */
+BENCHMARK_DEFINE_F(TracezProcessor, BM_SnapComplete)(benchmark::State &state)
 {
-  while(state.KeepRunningBatch(numIterations))
+  for (auto _ : state)
   {
-    StartManySameSpans(spans, tracer, 500);
-    EndSpans(spans);
+    StartManySpans(spans, tracer, numSpans);
+
+    std::thread snapshots(GetManySnapshots, std::ref(processor), numSpans);
+    std::thread end(EndAllSpans, std::ref(spans));
+
+    snapshots.join();
+    end.join();
   }
+
 }
 
+/*
+ * Make many empty spans and end them, all while snapshots are being grabbed.
+ */
+BENCHMARK_DEFINE_F(TracezProcessor, BM_RunSnapComplete)(benchmark::State &state)
+{
+  for (auto _ : state)
+  {
+    std::vector<opentelemetry::nostd::shared_ptr<opentelemetry::trace::Span>> spans2;
+
+    StartManySpans(spans, tracer, numSpans);
+
+    std::thread start(StartManySpans, std::ref(spans2), tracer, numSpans);
+    std::thread snapshots(GetManySnapshots, std::ref(processor), numSpans);
+    std::thread end(EndAllSpans, std::ref(spans));
+
+    start.join();
+    snapshots.join();
+    end.join();
+
+    EndAllSpans(spans2);
+  }
+}
+
+/////////////////////// RUN BENCHMARKS ///////////////////////////
+
+BENCHMARK_REGISTER_F(TracezProcessor, BM_RunComplete)->Arg(10)->Arg(1000);
+BENCHMARK_REGISTER_F(TracezProcessor, BM_RunSnap)->Arg(10)->Arg(1000);
+BENCHMARK_REGISTER_F(TracezProcessor, BM_SnapComplete)->Arg(10)->Arg(1000);
+BENCHMARK_REGISTER_F(TracezProcessor, BM_RunSnapComplete)->Arg(10)->Arg(1000);
 
 BENCHMARK_MAIN();
+

From 44c9c426ef4e428e2f05a7a30f8e21f3ae0a3a59 Mon Sep 17 00:00:00 2001
From: Janet Vu <vjanet@umich.edu>
Date: Fri, 21 Aug 2020 19:06:16 +0000
Subject: [PATCH 03/40] Add aggregator benchmarks, use processor

---
 ext/test/zpages/BUILD                         |  12 ++
 .../tracez_data_aggregator_benchmark.cc       | 154 ++++++++++++++++++
 2 files changed, 166 insertions(+)
 create mode 100644 ext/test/zpages/tracez_data_aggregator_benchmark.cc

diff --git a/ext/test/zpages/BUILD b/ext/test/zpages/BUILD
index b7177b2d57..c4d6719cf7 100644
--- a/ext/test/zpages/BUILD
+++ b/ext/test/zpages/BUILD
@@ -36,6 +36,18 @@ cc_test(
     ],
 )
 
+otel_cc_benchmark(
+    name = "tracez_aggregator_benchmark",
+    srcs = [
+        "tracez_data_aggregator_benchmark.cc",
+    ],
+    deps = [
+        "//ext/src/zpages",
+        "//sdk/src/trace",
+        "//api",
+    ],
+)
+
 otel_cc_benchmark(
     name = "tracez_processor_benchmark",
     srcs = [
diff --git a/ext/test/zpages/tracez_data_aggregator_benchmark.cc b/ext/test/zpages/tracez_data_aggregator_benchmark.cc
new file mode 100644
index 0000000000..2c4fca681a
--- /dev/null
+++ b/ext/test/zpages/tracez_data_aggregator_benchmark.cc
@@ -0,0 +1,154 @@
+#include "opentelemetry/context/threadlocal_context.h"
+#include "opentelemetry/ext/zpages/tracez_data_aggregator.h"
+
+#include <benchmark/benchmark.h>
+#include <chrono>
+
+#include "opentelemetry/sdk/trace/tracer.h"
+
+using namespace opentelemetry::sdk::trace;
+using namespace opentelemetry::ext::zpages;
+using opentelemetry::core::SteadyTimestamp;
+
+/////////////////////////////// BENCHMARK HELPER FUNCTIONS //////////////////////////////
+
+/*
+ * Helper function that creates and ends i spans instantly
+ */
+void StartEndSpans(
+    std::shared_ptr<opentelemetry::trace::Tracer> tracer,
+    int i,
+    bool isUnique = false)
+{
+  for (; i > 0; i--)
+    tracer->StartSpan(isUnique ? std::to_string(i) : "")->End();
+}
+
+/*
+ * Helper function that creates and ends spans instantly, while simulating
+ * different latencies.
+ */
+void StartEndSpansLatency(
+    std::shared_ptr<opentelemetry::trace::Tracer> tracer,
+    int i,
+    bool isUnique = false)
+{
+  opentelemetry::trace::StartSpanOptions start;
+  start.start_steady_time = SteadyTimestamp(nanoseconds(0));
+  for (; i > 0; i--)
+  {
+    opentelemetry::trace::EndSpanOptions end;
+    end.end_steady_time = SteadyTimestamp(kLatencyBoundaries[i % kLatencyBoundaries.size()]);
+    tracer->StartSpan(isUnique ? std::to_string(i) : "", start)->End(end);
+  }
+}
+/*
+ * Helper function that creates and ends spans instantly. while simulating
+ * error codes.
+ */
+void StartEndSpansError(
+    std::shared_ptr<opentelemetry::trace::Tracer> tracer,
+    int i,
+    bool isUnique = false)
+{
+  for (; i > 0; i--)
+    tracer->StartSpan(isUnique ? std::to_string(i) : "")
+        ->SetStatus(opentelemetry::trace::CanonicalCode::CANCELLED, "");
+}
+
+/*
+ * Helper function that creates i spans that always run.
+ */
+void StartSpans(
+    std::vector<opentelemetry::nostd::shared_ptr<opentelemetry::trace::Span>> &spans,
+    std::shared_ptr<opentelemetry::trace::Tracer> tracer,
+    int i,
+    bool isUnique = false)
+{
+  for (; i > 0; i--)
+    spans.push_back(tracer->StartSpan(isUnique ? std::to_string(i) : ""));
+}
+
+////////////////////////  FIXTURE FOR SHARED SETUP CODE ///////////////////
+
+class TracezAggregator : public benchmark::Fixture
+{
+protected:
+  void SetUp(const ::benchmark::State& state)
+  {
+    std::shared_ptr<TracezSpanProcessor> processor(new TracezSpanProcessor());
+    tracer                 = std::shared_ptr<opentelemetry::trace::Tracer>(new Tracer(processor));
+    tracez_data_aggregator = std::unique_ptr<TracezDataAggregator>(
+        new TracezDataAggregator(processor, milliseconds(10)));
+  }
+
+  std::unique_ptr<TracezDataAggregator> tracez_data_aggregator;
+  std::shared_ptr<opentelemetry::trace::Tracer> tracer;
+
+  const int numSpans      = 1000;
+};
+
+//////////////////////////// BENCHMARK DEFINITIONS /////////////////////////////////
+
+/*
+ * Aggregator handing many spans with the same name, who end instantly.
+ */
+
+BENCHMARK_DEFINE_F(TracezAggregator, BM_InstantSame)(benchmark::State &state)
+{
+  for (auto _ : state)
+  {
+    StartEndSpans(tracer, numSpans);
+  }
+}
+
+/*
+ * Aggregator handing many spans with unique names, who end instantly.
+ */
+
+BENCHMARK_DEFINE_F(TracezAggregator, BM_InstantUnique)(benchmark::State &state)
+{
+  for (auto _ : state)
+  {
+    StartEndSpans(tracer, numSpans, true);
+  }
+}
+
+/*
+ * Aggregator handing many spans with the same name, who end instantly.
+ */
+
+BENCHMARK_DEFINE_F(TracezAggregator, BM_BucketsSame)(benchmark::State &state)
+{
+  for (auto _ : state)
+  {
+    std::vector<opentelemetry::nostd::shared_ptr<opentelemetry::trace::Span>> running_spans;
+    StartSpans(running_spans, tracer, numSpans / 3);
+    StartEndSpansError(tracer, numSpans / 3);
+    StartEndSpansLatency(tracer, numSpans / 3);
+  }
+}
+
+/*
+ * Aggregator handing many spans with unique names, who end instantly.
+ */
+
+BENCHMARK_DEFINE_F(TracezAggregator, BM_BucketsUnique)(benchmark::State &state)
+{
+  for (auto _ : state)
+  {
+    std::vector<opentelemetry::nostd::shared_ptr<opentelemetry::trace::Span>> running_spans;
+    StartSpans(running_spans, tracer, numSpans / 3, true);
+    StartEndSpansError(tracer, numSpans / 3, true);
+    StartEndSpansLatency(tracer, numSpans / 3, true);
+  }
+}
+/////////////////////// RUN BENCHMARKS ///////////////////////////
+
+BENCHMARK_REGISTER_F(TracezAggregator, BM_InstantSame)->Arg(10)->Arg(1000);
+BENCHMARK_REGISTER_F(TracezAggregator, BM_InstantUnique)->Arg(10)->Arg(1000);
+BENCHMARK_REGISTER_F(TracezAggregator, BM_BucketsSame)->Arg(10)->Arg(1000);
+BENCHMARK_REGISTER_F(TracezAggregator, BM_BucketsUnique)->Arg(10)->Arg(1000);
+
+BENCHMARK_MAIN();
+

From 9088fd6177cdb537d4052b625ec84a97060f950c Mon Sep 17 00:00:00 2001
From: Janet Vu <vjanet@umich.edu>
Date: Fri, 21 Aug 2020 19:41:17 +0000
Subject: [PATCH 04/40] Move latency bound to separate var

---
 ext/test/zpages/tracez_data_aggregator_benchmark.cc | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/ext/test/zpages/tracez_data_aggregator_benchmark.cc b/ext/test/zpages/tracez_data_aggregator_benchmark.cc
index 2c4fca681a..8b506a5fdd 100644
--- a/ext/test/zpages/tracez_data_aggregator_benchmark.cc
+++ b/ext/test/zpages/tracez_data_aggregator_benchmark.cc
@@ -37,8 +37,11 @@ void StartEndSpansLatency(
   start.start_steady_time = SteadyTimestamp(nanoseconds(0));
   for (; i > 0; i--)
   {
+    // Latency bucket depends on the index
+    auto latency_band = kLatencyBoundaries[i % kLatencyBoundaries.size()];
     opentelemetry::trace::EndSpanOptions end;
-    end.end_steady_time = SteadyTimestamp(kLatencyBoundaries[i % kLatencyBoundaries.size()]);
+    end.end_steady_time = SteadyTimestamp(latency_band);
+
     tracer->StartSpan(isUnique ? std::to_string(i) : "", start)->End(end);
   }
 }

From 75ab9424aaa9dc0f1d6431ae7bdf6b5d08461662 Mon Sep 17 00:00:00 2001
From: Janet Vu <vjanet@umich.edu>
Date: Sat, 22 Aug 2020 18:30:04 +0000
Subject: [PATCH 05/40] Restructure ext folder

---
 examples/zpages/BUILD                         |  3 +-
 ext/CMakeLists.txt                            |  6 ++--
 ext/http/CMakeLists.txt                       |  2 ++
 ext/{ => http/server}/BUILD                   |  0
 ext/http/server/CMakeLists.txt                |  4 +++
 .../ext/http/server/file_http_server.h        |  0
 .../ext/http/server/http_server.h             |  0
 .../ext/http/server/socket_tools.h            |  0
 ext/src/CMakeLists.txt                        |  1 -
 ext/src/zpages/BUILD                          | 28 -------------------
 ext/src/zpages/CMakeLists.txt                 | 12 --------
 ext/test/CMakeLists.txt                       |  1 -
 ext/zpages/BUILD                              | 15 ++++++++++
 ext/zpages/CMakeLists.txt                     | 15 ++++++++++
 .../ext/zpages/latency_boundaries.h           |  0
 .../ext/zpages/static/tracez_index.h          |  0
 .../ext/zpages/static/tracez_script.h         |  0
 .../ext/zpages/static/tracez_style.h          |  0
 .../ext/zpages/threadsafe_span_data.h         |  0
 .../opentelemetry/ext/zpages/tracez_data.h    |  0
 .../ext/zpages/tracez_data_aggregator.h       |  0
 .../ext/zpages/tracez_http_server.h           |  0
 .../ext/zpages/tracez_processor.h             |  0
 .../include/opentelemetry/ext/zpages/zpages.h |  0
 .../ext/zpages/zpages_http_server.h           |  0
 ext/{src/zpages => zpages/src}/README.md      |  0
 .../src}/tracez_data_aggregator.cc            |  0
 .../src}/tracez_http_server.cc                |  0
 .../zpages => zpages/src}/tracez_processor.cc |  0
 ext/{test/zpages => zpages/test}/BUILD        |  6 ++--
 .../zpages => zpages/test}/CMakeLists.txt     |  0
 .../test}/threadsafe_span_data_test.cc        |  0
 .../test}/tracez_data_aggregator_test.cc      |  0
 .../test}/tracez_processor_test.cc            |  0
 34 files changed, 42 insertions(+), 51 deletions(-)
 create mode 100644 ext/http/CMakeLists.txt
 rename ext/{ => http/server}/BUILD (100%)
 create mode 100644 ext/http/server/CMakeLists.txt
 rename ext/{ => http/server}/include/opentelemetry/ext/http/server/file_http_server.h (100%)
 rename ext/{ => http/server}/include/opentelemetry/ext/http/server/http_server.h (100%)
 rename ext/{ => http/server}/include/opentelemetry/ext/http/server/socket_tools.h (100%)
 delete mode 100644 ext/src/CMakeLists.txt
 delete mode 100644 ext/src/zpages/BUILD
 delete mode 100644 ext/src/zpages/CMakeLists.txt
 delete mode 100644 ext/test/CMakeLists.txt
 create mode 100644 ext/zpages/BUILD
 create mode 100644 ext/zpages/CMakeLists.txt
 rename ext/{ => zpages}/include/opentelemetry/ext/zpages/latency_boundaries.h (100%)
 rename ext/{ => zpages}/include/opentelemetry/ext/zpages/static/tracez_index.h (100%)
 rename ext/{ => zpages}/include/opentelemetry/ext/zpages/static/tracez_script.h (100%)
 rename ext/{ => zpages}/include/opentelemetry/ext/zpages/static/tracez_style.h (100%)
 rename ext/{ => zpages}/include/opentelemetry/ext/zpages/threadsafe_span_data.h (100%)
 rename ext/{ => zpages}/include/opentelemetry/ext/zpages/tracez_data.h (100%)
 rename ext/{ => zpages}/include/opentelemetry/ext/zpages/tracez_data_aggregator.h (100%)
 rename ext/{ => zpages}/include/opentelemetry/ext/zpages/tracez_http_server.h (100%)
 rename ext/{ => zpages}/include/opentelemetry/ext/zpages/tracez_processor.h (100%)
 rename ext/{ => zpages}/include/opentelemetry/ext/zpages/zpages.h (100%)
 rename ext/{ => zpages}/include/opentelemetry/ext/zpages/zpages_http_server.h (100%)
 rename ext/{src/zpages => zpages/src}/README.md (100%)
 rename ext/{src/zpages => zpages/src}/tracez_data_aggregator.cc (100%)
 rename ext/{src/zpages => zpages/src}/tracez_http_server.cc (100%)
 rename ext/{src/zpages => zpages/src}/tracez_processor.cc (100%)
 rename ext/{test/zpages => zpages/test}/BUILD (88%)
 rename ext/{test/zpages => zpages/test}/CMakeLists.txt (100%)
 rename ext/{test/zpages => zpages/test}/threadsafe_span_data_test.cc (100%)
 rename ext/{test/zpages => zpages/test}/tracez_data_aggregator_test.cc (100%)
 rename ext/{test/zpages => zpages/test}/tracez_processor_test.cc (100%)

diff --git a/examples/zpages/BUILD b/examples/zpages/BUILD
index a277c44810..778c7ffe3a 100644
--- a/examples/zpages/BUILD
+++ b/examples/zpages/BUILD
@@ -24,8 +24,7 @@ cc_binary(
         "//conditions:default": ["-pthread"],
     }),
     deps = [
-        "//ext:headers",
-        "//ext/src/zpages",
+        "//ext/zpages",
         "//sdk/src/trace",
     ],
 )
diff --git a/ext/CMakeLists.txt b/ext/CMakeLists.txt
index 75205ac71e..1b46c53a08 100644
--- a/ext/CMakeLists.txt
+++ b/ext/CMakeLists.txt
@@ -1,5 +1,3 @@
-add_subdirectory(src)
+add_subdirectory(http)
+add_subdirectory(zpages)
 
-if(BUILD_TESTING)
-  add_subdirectory(test)
-endif()
diff --git a/ext/http/CMakeLists.txt b/ext/http/CMakeLists.txt
new file mode 100644
index 0000000000..b0531d9ab6
--- /dev/null
+++ b/ext/http/CMakeLists.txt
@@ -0,0 +1,2 @@
+add_subdirectory(server)
+
diff --git a/ext/BUILD b/ext/http/server/BUILD
similarity index 100%
rename from ext/BUILD
rename to ext/http/server/BUILD
diff --git a/ext/http/server/CMakeLists.txt b/ext/http/server/CMakeLists.txt
new file mode 100644
index 0000000000..cd4d2cdc4e
--- /dev/null
+++ b/ext/http/server/CMakeLists.txt
@@ -0,0 +1,4 @@
+add_library(http_server INTERFACE)
+
+target_include_directories(http_server INTERFACE include)
+
diff --git a/ext/include/opentelemetry/ext/http/server/file_http_server.h b/ext/http/server/include/opentelemetry/ext/http/server/file_http_server.h
similarity index 100%
rename from ext/include/opentelemetry/ext/http/server/file_http_server.h
rename to ext/http/server/include/opentelemetry/ext/http/server/file_http_server.h
diff --git a/ext/include/opentelemetry/ext/http/server/http_server.h b/ext/http/server/include/opentelemetry/ext/http/server/http_server.h
similarity index 100%
rename from ext/include/opentelemetry/ext/http/server/http_server.h
rename to ext/http/server/include/opentelemetry/ext/http/server/http_server.h
diff --git a/ext/include/opentelemetry/ext/http/server/socket_tools.h b/ext/http/server/include/opentelemetry/ext/http/server/socket_tools.h
similarity index 100%
rename from ext/include/opentelemetry/ext/http/server/socket_tools.h
rename to ext/http/server/include/opentelemetry/ext/http/server/socket_tools.h
diff --git a/ext/src/CMakeLists.txt b/ext/src/CMakeLists.txt
deleted file mode 100644
index 189a03f69c..0000000000
--- a/ext/src/CMakeLists.txt
+++ /dev/null
@@ -1 +0,0 @@
-add_subdirectory(zpages)
diff --git a/ext/src/zpages/BUILD b/ext/src/zpages/BUILD
deleted file mode 100644
index ab9f3507b8..0000000000
--- a/ext/src/zpages/BUILD
+++ /dev/null
@@ -1,28 +0,0 @@
-# Copyright 2020, OpenTelemetry 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(default_visibility = ["//visibility:public"])
-
-cc_library(
-    name = "zpages",
-    srcs = glob(["**/*.cc"]),
-    hdrs = glob(["**/*.h"]),
-    include_prefix = "ext/zpages",
-    deps = [
-        "//api",
-        "//ext:headers",
-        "//sdk:headers",
-        "@github_nlohmann_json//:json",
-    ],
-)
diff --git a/ext/src/zpages/CMakeLists.txt b/ext/src/zpages/CMakeLists.txt
deleted file mode 100644
index dae0249bf4..0000000000
--- a/ext/src/zpages/CMakeLists.txt
+++ /dev/null
@@ -1,12 +0,0 @@
-add_library(
-  opentelemetry_zpages
-  tracez_processor.cc
-  tracez_data_aggregator.cc
-  ../../include/opentelemetry/ext/zpages/tracez_processor.h
-  ../../include/opentelemetry/ext/zpages/tracez_data_aggregator.h
-  ../../include/opentelemetry/ext/zpages/tracez_http_server.h)
-
-target_include_directories(opentelemetry_zpages PUBLIC ../../include)
-
-target_link_libraries(opentelemetry_zpages opentelemetry_api
-                      opentelemetry_trace)
diff --git a/ext/test/CMakeLists.txt b/ext/test/CMakeLists.txt
deleted file mode 100644
index 189a03f69c..0000000000
--- a/ext/test/CMakeLists.txt
+++ /dev/null
@@ -1 +0,0 @@
-add_subdirectory(zpages)
diff --git a/ext/zpages/BUILD b/ext/zpages/BUILD
new file mode 100644
index 0000000000..df96971b35
--- /dev/null
+++ b/ext/zpages/BUILD
@@ -0,0 +1,15 @@
+package(default_visibility = ["//visibility:public"])
+
+cc_library(
+    name = "zpages",
+    srcs = glob(["src/**/*.cc"]),
+    hdrs = glob(["include/**/*.h"]),
+    strip_include_prefix = "include",
+    deps = [
+        "//api",
+        "//ext/http/server:headers",
+        "//sdk:headers",
+        "@github_nlohmann_json//:json",
+    ],
+)
+
diff --git a/ext/zpages/CMakeLists.txt b/ext/zpages/CMakeLists.txt
new file mode 100644
index 0000000000..62c3976867
--- /dev/null
+++ b/ext/zpages/CMakeLists.txt
@@ -0,0 +1,15 @@
+add_library(
+  opentelemetry_zpages
+  src/tracez_processor.cc
+  src/tracez_data_aggregator.cc
+  src/tracez_http_server.cc
+)
+
+target_include_directories(opentelemetry_zpages PUBLIC include)
+
+target_link_libraries(opentelemetry_zpages opentelemetry_api
+                      opentelemetry_trace http_server)
+
+if(BUILD_TESTING)
+  add_subdirectory(test)
+endif()
diff --git a/ext/include/opentelemetry/ext/zpages/latency_boundaries.h b/ext/zpages/include/opentelemetry/ext/zpages/latency_boundaries.h
similarity index 100%
rename from ext/include/opentelemetry/ext/zpages/latency_boundaries.h
rename to ext/zpages/include/opentelemetry/ext/zpages/latency_boundaries.h
diff --git a/ext/include/opentelemetry/ext/zpages/static/tracez_index.h b/ext/zpages/include/opentelemetry/ext/zpages/static/tracez_index.h
similarity index 100%
rename from ext/include/opentelemetry/ext/zpages/static/tracez_index.h
rename to ext/zpages/include/opentelemetry/ext/zpages/static/tracez_index.h
diff --git a/ext/include/opentelemetry/ext/zpages/static/tracez_script.h b/ext/zpages/include/opentelemetry/ext/zpages/static/tracez_script.h
similarity index 100%
rename from ext/include/opentelemetry/ext/zpages/static/tracez_script.h
rename to ext/zpages/include/opentelemetry/ext/zpages/static/tracez_script.h
diff --git a/ext/include/opentelemetry/ext/zpages/static/tracez_style.h b/ext/zpages/include/opentelemetry/ext/zpages/static/tracez_style.h
similarity index 100%
rename from ext/include/opentelemetry/ext/zpages/static/tracez_style.h
rename to ext/zpages/include/opentelemetry/ext/zpages/static/tracez_style.h
diff --git a/ext/include/opentelemetry/ext/zpages/threadsafe_span_data.h b/ext/zpages/include/opentelemetry/ext/zpages/threadsafe_span_data.h
similarity index 100%
rename from ext/include/opentelemetry/ext/zpages/threadsafe_span_data.h
rename to ext/zpages/include/opentelemetry/ext/zpages/threadsafe_span_data.h
diff --git a/ext/include/opentelemetry/ext/zpages/tracez_data.h b/ext/zpages/include/opentelemetry/ext/zpages/tracez_data.h
similarity index 100%
rename from ext/include/opentelemetry/ext/zpages/tracez_data.h
rename to ext/zpages/include/opentelemetry/ext/zpages/tracez_data.h
diff --git a/ext/include/opentelemetry/ext/zpages/tracez_data_aggregator.h b/ext/zpages/include/opentelemetry/ext/zpages/tracez_data_aggregator.h
similarity index 100%
rename from ext/include/opentelemetry/ext/zpages/tracez_data_aggregator.h
rename to ext/zpages/include/opentelemetry/ext/zpages/tracez_data_aggregator.h
diff --git a/ext/include/opentelemetry/ext/zpages/tracez_http_server.h b/ext/zpages/include/opentelemetry/ext/zpages/tracez_http_server.h
similarity index 100%
rename from ext/include/opentelemetry/ext/zpages/tracez_http_server.h
rename to ext/zpages/include/opentelemetry/ext/zpages/tracez_http_server.h
diff --git a/ext/include/opentelemetry/ext/zpages/tracez_processor.h b/ext/zpages/include/opentelemetry/ext/zpages/tracez_processor.h
similarity index 100%
rename from ext/include/opentelemetry/ext/zpages/tracez_processor.h
rename to ext/zpages/include/opentelemetry/ext/zpages/tracez_processor.h
diff --git a/ext/include/opentelemetry/ext/zpages/zpages.h b/ext/zpages/include/opentelemetry/ext/zpages/zpages.h
similarity index 100%
rename from ext/include/opentelemetry/ext/zpages/zpages.h
rename to ext/zpages/include/opentelemetry/ext/zpages/zpages.h
diff --git a/ext/include/opentelemetry/ext/zpages/zpages_http_server.h b/ext/zpages/include/opentelemetry/ext/zpages/zpages_http_server.h
similarity index 100%
rename from ext/include/opentelemetry/ext/zpages/zpages_http_server.h
rename to ext/zpages/include/opentelemetry/ext/zpages/zpages_http_server.h
diff --git a/ext/src/zpages/README.md b/ext/zpages/src/README.md
similarity index 100%
rename from ext/src/zpages/README.md
rename to ext/zpages/src/README.md
diff --git a/ext/src/zpages/tracez_data_aggregator.cc b/ext/zpages/src/tracez_data_aggregator.cc
similarity index 100%
rename from ext/src/zpages/tracez_data_aggregator.cc
rename to ext/zpages/src/tracez_data_aggregator.cc
diff --git a/ext/src/zpages/tracez_http_server.cc b/ext/zpages/src/tracez_http_server.cc
similarity index 100%
rename from ext/src/zpages/tracez_http_server.cc
rename to ext/zpages/src/tracez_http_server.cc
diff --git a/ext/src/zpages/tracez_processor.cc b/ext/zpages/src/tracez_processor.cc
similarity index 100%
rename from ext/src/zpages/tracez_processor.cc
rename to ext/zpages/src/tracez_processor.cc
diff --git a/ext/test/zpages/BUILD b/ext/zpages/test/BUILD
similarity index 88%
rename from ext/test/zpages/BUILD
rename to ext/zpages/test/BUILD
index 07c287d2fa..939a7e770f 100644
--- a/ext/test/zpages/BUILD
+++ b/ext/zpages/test/BUILD
@@ -4,7 +4,7 @@ cc_test(
         "threadsafe_span_data_test.cc",
     ],
     deps = [
-        "//ext/src/zpages",
+        "//ext/zpages",
         "//sdk/src/trace",
         "@com_google_googletest//:gtest_main",
     ],
@@ -16,7 +16,7 @@ cc_test(
         "tracez_data_aggregator_test.cc",
     ],
     deps = [
-        "//ext/src/zpages",
+        "//ext/zpages",
         "//sdk/src/trace",
         "@com_google_googletest//:gtest_main",
     ],
@@ -28,7 +28,7 @@ cc_test(
         "tracez_processor_test.cc",
     ],
     deps = [
-        "//ext/src/zpages",
+        "//ext/zpages",
         "//sdk/src/trace",
         "@com_google_googletest//:gtest_main",
     ],
diff --git a/ext/test/zpages/CMakeLists.txt b/ext/zpages/test/CMakeLists.txt
similarity index 100%
rename from ext/test/zpages/CMakeLists.txt
rename to ext/zpages/test/CMakeLists.txt
diff --git a/ext/test/zpages/threadsafe_span_data_test.cc b/ext/zpages/test/threadsafe_span_data_test.cc
similarity index 100%
rename from ext/test/zpages/threadsafe_span_data_test.cc
rename to ext/zpages/test/threadsafe_span_data_test.cc
diff --git a/ext/test/zpages/tracez_data_aggregator_test.cc b/ext/zpages/test/tracez_data_aggregator_test.cc
similarity index 100%
rename from ext/test/zpages/tracez_data_aggregator_test.cc
rename to ext/zpages/test/tracez_data_aggregator_test.cc
diff --git a/ext/test/zpages/tracez_processor_test.cc b/ext/zpages/test/tracez_processor_test.cc
similarity index 100%
rename from ext/test/zpages/tracez_processor_test.cc
rename to ext/zpages/test/tracez_processor_test.cc

From d67f20ed92d7950b02aa2c8e85aaec36ffdc0034 Mon Sep 17 00:00:00 2001
From: Janet Vu <vjanet@umich.edu>
Date: Sat, 22 Aug 2020 19:01:10 +0000
Subject: [PATCH 06/40] Compile CMake correctly, json cmake todo

---
 ext/zpages/CMakeLists.txt | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/ext/zpages/CMakeLists.txt b/ext/zpages/CMakeLists.txt
index 62c3976867..0b245b10fa 100644
--- a/ext/zpages/CMakeLists.txt
+++ b/ext/zpages/CMakeLists.txt
@@ -2,7 +2,8 @@ add_library(
   opentelemetry_zpages
   src/tracez_processor.cc
   src/tracez_data_aggregator.cc
-  src/tracez_http_server.cc
+  # TODO: add Cmake build for nlohmann JSON
+  # src/tracez_http_server.cc
 )
 
 target_include_directories(opentelemetry_zpages PUBLIC include)

From d05de6c88d47f579568545e05f9c8165c8b5922a Mon Sep 17 00:00:00 2001
From: Janet Vu <vjanet@umich.edu>
Date: Sat, 22 Aug 2020 19:04:32 +0000
Subject: [PATCH 07/40] Move and update zPages README

---
 ext/zpages/README.md     | 27 ++++++++++++++++++++++++
 ext/zpages/src/README.md | 45 ----------------------------------------
 2 files changed, 27 insertions(+), 45 deletions(-)
 create mode 100644 ext/zpages/README.md
 delete mode 100644 ext/zpages/src/README.md

diff --git a/ext/zpages/README.md b/ext/zpages/README.md
new file mode 100644
index 0000000000..2ca715ed77
--- /dev/null
+++ b/ext/zpages/README.md
@@ -0,0 +1,27 @@
+# zPages
+## Overview
+zPages are a quick and light way to view tracing and metrics information on standard OpenTelemetry C++ instrumented applications. It requires no external dependencies. See more information in the OTel zPages experimental [spec](https://github.com/open-telemetry/opentelemetry-specification/blob/5b86d4b6c42e6d1e47d9155ac1e2e27f0f0b7769/experimental/trace/zpages.md).
+
+# Usage
+> TODO: Add CMake instructions
+
+1. Add the following 2 lines of code
+ - `#include opentelemetry/ext/zpages/zpages.h // include zPages`
+ - `zpages::Initialize; // start up zPages in your app, before any tracing/span code`
+2. Build and run your application normally
+ - For example, you can do this for the zPages example while at the root `opentelemetry-cpp` directory with:
+   ```
+   bazel build //examples/zpages:zpages_example
+   bazel-bin/examples/zpages/zpages_example
+   ```
+   If you look at the zPages example's source [code](https://github.com/open-telemetry/opentelemetry-cpp/blob/master/examples/zpages/zpages_example.cc), it demonstrates adding zPages, manual application instrumentation (which sends data to zPages for viewing), and simulated use cases for zPages.
+3. View zPages ib http://localhost:3000/tracez
+
+
+## More Information
+- OTel zPages experimental [spec](https://github.com/open-telemetry/opentelemetry-specification/blob/5b86d4b6c42e6d1e47d9155ac1e2e27f0f0b7769/experimental/trace/zpages.md)
+- [zPages General Direction Spec (OTEP)](https://github.com/open-telemetry/oteps/blob/master/text/0110-z-pages.md)
+- OTel C++ Design Docs
+  - [Tracez Span Processor](https://docs.google.com/document/d/1kO4iZARYyr-EGBlY2VNM3ELU3iw6ZrC58Omup_YT-fU/edit#)
+  - [Tracez Data Aggregator](https://docs.google.com/document/d/1ziKFgvhXFfRXZjOlAHQRR-TzcNcTXzg1p2I9oPCEIoU/edit?ts=5ef0d177#heading=h.5irk4csrpu0y)
+  - [Tracez Http Server](https://docs.google.com/document/d/1U1V8QZ5LtGl4Mich-aJ6KZGLHrMIE8pWyspmzvnIefI/edit#) - also includes reference pictures
diff --git a/ext/zpages/src/README.md b/ext/zpages/src/README.md
deleted file mode 100644
index 8e8fed394a..0000000000
--- a/ext/zpages/src/README.md
+++ /dev/null
@@ -1,45 +0,0 @@
-# zPages
-
-## Table of Contents
-- [Summary](#summary)
-  - [TraceZ](#tracez)
-  - [RPCz](#rpcz)
-- [Usage](#usage)
-- [Links of Interest](#links-of-interest)
-
-## Summary
-zPages allow easy viewing of tracing information. When included for a process, zPages will display basic information about that process on a webpage. Two types of zPages include TraceZ and RPCz.
-
-Including a zPage within a page is useful for developers because it's quicker to get running than adding extra code and installing external exporters like Jaeger and Zipkin. zPages tend to be more lightweight than these external exporters, but are also helpful for debugging latency issues (slow parts of applications) and deadlocks (running spans that don't end).
-
-The idea of "zPages" originates from one of OpenTelemetry's predecessors, [OpenCensus](https://opencensus.io/). You can read more about it [here](https://opencensus.io/zpages). OpenCensus has different zPage implementations in [Java](https://opencensus.io/zpages/java/), [Go](https://opencensus.io/zpages/go/), and [Node](https://opencensus.io/zpages/node/) and there has been similar internal solutions developed at companies like Uber, but *this is the first major open-source implementation of zPages in C++*. Within OpenTelemetry, zPages are also being developed in [Java](https://github.com/open-telemetry/opentelemetry-java).
-
-#### How It Works
-On a high level, an application creates spans using a Tracer Provider/Tracer who passes them to a Span Processor, which exports spans to the appropriate Data Aggregator that a Http Server displays information for.
-
-> TODO: Add picture examples for span overview and individual span view
-
-### TraceZ
-TraceZ is a type of zPage that shows information on tracing spans, and allows users to look closer at specific and individual spans. Details a user would view include span id, name, status, and timestamps. The individual components of TraceZ are as follows:
-
-- TracezSpanProcessor (TSP)
-  - A tracer/tracer provider (which the user chooses) creates spans, which connects to TSP so that TraceZ to detect spans. The TSP then stores tracing information in running and completed containers and provides an interface for TDA to access their information.
-- TracezDataAggregator (TDA)
-  - Intermediary between the TSP and THS, which also performs various functions and calculations (mainly grouping spans by their names and latency times) to send the correct tracing information to the THS.
-- TracezHttpServer (THS)
-  - User-facing web page generator, which creates HTML pages using TDA that display 1) overall information and trends on all of the process's spans and 2) more detailed information on specific spans when clicked.
-
-### RPCz
-RPCz is a type of zPage that provides details on instrumented sent and received RPC messages. Although there is currently no ongoing development of RPCz for OpenTelemetry, OpenCensus zPages have implementations of RPCz (linked above).
-
-# Usage
-
-> TODO: Add instructions to add zPages
-
-## Links of Interest
-- [TracezSpanProcessor Design Doc](https://docs.google.com/document/d/1kO4iZARYyr-EGBlY2VNM3ELU3iw6ZrC58Omup_YT-fU/edit#) (pending review)
-- [TracezDataAggregator Design Doc](https://docs.google.com/document/d/1ziKFgvhXFfRXZjOlAHQRR-TzcNcTXzg1p2I9oPCEIoU/edit?ts=5ef0d177#heading=h.5irk4csrpu0y) (pending review)
-- [TracezHttpServer Design Doc](https://docs.google.com/document/d/1U1V8QZ5LtGl4Mich-aJ6KZGLHrMIE8pWyspmzvnIefI/edit#) (draft)
-- [zPages General Direction Spec](https://github.com/open-telemetry/oteps/blob/master/text/0110-z-pages.md)
-- [Prospective Fields Displayed by
-  TraceZ](https://github.com/open-telemetry/opentelemetry-cpp/blob/master/sdk/include/opentelemetry/sdk/trace/span_data.h)

From 69c72d9b8e97725839e4d0729282b0880e33e1d1 Mon Sep 17 00:00:00 2001
From: Janet Vu <vjanet@umich.edu>
Date: Sat, 22 Aug 2020 19:22:51 +0000
Subject: [PATCH 08/40] Update README to whats in #288

---
 ext/zpages/README.md | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/ext/zpages/README.md b/ext/zpages/README.md
index 2ca715ed77..40df3d94f1 100644
--- a/ext/zpages/README.md
+++ b/ext/zpages/README.md
@@ -24,4 +24,5 @@ zPages are a quick and light way to view tracing and metrics information on stan
 - OTel C++ Design Docs
   - [Tracez Span Processor](https://docs.google.com/document/d/1kO4iZARYyr-EGBlY2VNM3ELU3iw6ZrC58Omup_YT-fU/edit#)
   - [Tracez Data Aggregator](https://docs.google.com/document/d/1ziKFgvhXFfRXZjOlAHQRR-TzcNcTXzg1p2I9oPCEIoU/edit?ts=5ef0d177#heading=h.5irk4csrpu0y)
-  - [Tracez Http Server](https://docs.google.com/document/d/1U1V8QZ5LtGl4Mich-aJ6KZGLHrMIE8pWyspmzvnIefI/edit#) - also includes reference pictures
+  - [Tracez Http Server](https://docs.google.com/document/d/1U1V8QZ5LtGl4Mich-aJ6KZGLHrMIE8pWyspmzvnIefI/edit#) - also includes reference picturesof zPages/Tracez UI
+

From 64f4beabca8c671533ed53ed6e4d7f7f9de02b18 Mon Sep 17 00:00:00 2001
From: Janet Vu <vjanet@umich.edu>
Date: Sat, 22 Aug 2020 19:27:29 +0000
Subject: [PATCH 09/40] Run formatter

---
 ext/CMakeLists.txt             | 1 -
 ext/http/CMakeLists.txt        | 1 -
 ext/http/server/CMakeLists.txt | 1 -
 ext/zpages/BUILD               | 1 -
 ext/zpages/CMakeLists.txt      | 4 +---
 5 files changed, 1 insertion(+), 7 deletions(-)

diff --git a/ext/CMakeLists.txt b/ext/CMakeLists.txt
index 1b46c53a08..8d4e62acef 100644
--- a/ext/CMakeLists.txt
+++ b/ext/CMakeLists.txt
@@ -1,3 +1,2 @@
 add_subdirectory(http)
 add_subdirectory(zpages)
-
diff --git a/ext/http/CMakeLists.txt b/ext/http/CMakeLists.txt
index b0531d9ab6..9a84b352a6 100644
--- a/ext/http/CMakeLists.txt
+++ b/ext/http/CMakeLists.txt
@@ -1,2 +1 @@
 add_subdirectory(server)
-
diff --git a/ext/http/server/CMakeLists.txt b/ext/http/server/CMakeLists.txt
index cd4d2cdc4e..0b422b5d6a 100644
--- a/ext/http/server/CMakeLists.txt
+++ b/ext/http/server/CMakeLists.txt
@@ -1,4 +1,3 @@
 add_library(http_server INTERFACE)
 
 target_include_directories(http_server INTERFACE include)
-
diff --git a/ext/zpages/BUILD b/ext/zpages/BUILD
index df96971b35..4937379edc 100644
--- a/ext/zpages/BUILD
+++ b/ext/zpages/BUILD
@@ -12,4 +12,3 @@ cc_library(
         "@github_nlohmann_json//:json",
     ],
 )
-
diff --git a/ext/zpages/CMakeLists.txt b/ext/zpages/CMakeLists.txt
index 0b245b10fa..8690cbd3fd 100644
--- a/ext/zpages/CMakeLists.txt
+++ b/ext/zpages/CMakeLists.txt
@@ -1,7 +1,5 @@
 add_library(
-  opentelemetry_zpages
-  src/tracez_processor.cc
-  src/tracez_data_aggregator.cc
+  opentelemetry_zpages src/tracez_processor.cc src/tracez_data_aggregator.cc
   # TODO: add Cmake build for nlohmann JSON
   # src/tracez_http_server.cc
 )

From cfba6578cc7640f70864b52691f0d73f927d0625 Mon Sep 17 00:00:00 2001
From: Janet Vu <vjanet@umich.edu>
Date: Sat, 22 Aug 2020 20:10:47 +0000
Subject: [PATCH 10/40] Use args in benchmarks

---
 ext/zpages/test/tracez_data_aggregator_benchmark.cc | 6 ++++--
 ext/zpages/test/tracez_processor_benchmark.cc       | 6 ++++--
 2 files changed, 8 insertions(+), 4 deletions(-)

diff --git a/ext/zpages/test/tracez_data_aggregator_benchmark.cc b/ext/zpages/test/tracez_data_aggregator_benchmark.cc
index 8b506a5fdd..b3f8bda97c 100644
--- a/ext/zpages/test/tracez_data_aggregator_benchmark.cc
+++ b/ext/zpages/test/tracez_data_aggregator_benchmark.cc
@@ -87,8 +87,6 @@ class TracezAggregator : public benchmark::Fixture
 
   std::unique_ptr<TracezDataAggregator> tracez_data_aggregator;
   std::shared_ptr<opentelemetry::trace::Tracer> tracer;
-
-  const int numSpans      = 1000;
 };
 
 //////////////////////////// BENCHMARK DEFINITIONS /////////////////////////////////
@@ -99,6 +97,7 @@ class TracezAggregator : public benchmark::Fixture
 
 BENCHMARK_DEFINE_F(TracezAggregator, BM_InstantSame)(benchmark::State &state)
 {
+  const int numSpans = state.range(0);
   for (auto _ : state)
   {
     StartEndSpans(tracer, numSpans);
@@ -111,6 +110,7 @@ BENCHMARK_DEFINE_F(TracezAggregator, BM_InstantSame)(benchmark::State &state)
 
 BENCHMARK_DEFINE_F(TracezAggregator, BM_InstantUnique)(benchmark::State &state)
 {
+  const int numSpans = state.range(0);
   for (auto _ : state)
   {
     StartEndSpans(tracer, numSpans, true);
@@ -123,6 +123,7 @@ BENCHMARK_DEFINE_F(TracezAggregator, BM_InstantUnique)(benchmark::State &state)
 
 BENCHMARK_DEFINE_F(TracezAggregator, BM_BucketsSame)(benchmark::State &state)
 {
+  const int numSpans = state.range(0);
   for (auto _ : state)
   {
     std::vector<opentelemetry::nostd::shared_ptr<opentelemetry::trace::Span>> running_spans;
@@ -138,6 +139,7 @@ BENCHMARK_DEFINE_F(TracezAggregator, BM_BucketsSame)(benchmark::State &state)
 
 BENCHMARK_DEFINE_F(TracezAggregator, BM_BucketsUnique)(benchmark::State &state)
 {
+  const int numSpans = state.range(0);
   for (auto _ : state)
   {
     std::vector<opentelemetry::nostd::shared_ptr<opentelemetry::trace::Span>> running_spans;
diff --git a/ext/zpages/test/tracez_processor_benchmark.cc b/ext/zpages/test/tracez_processor_benchmark.cc
index 5aaac7656c..11710213c5 100644
--- a/ext/zpages/test/tracez_processor_benchmark.cc
+++ b/ext/zpages/test/tracez_processor_benchmark.cc
@@ -57,8 +57,6 @@ class TracezProcessor : public benchmark::Fixture
   std::shared_ptr<opentelemetry::trace::Tracer> tracer;
 
   std::vector<opentelemetry::nostd::shared_ptr<opentelemetry::trace::Span>> spans;
-
-  const int numSpans      = 50;
 };
 
 //////////////////////////// BENCHMARK DEFINITIONS /////////////////////////////////
@@ -69,6 +67,7 @@ class TracezProcessor : public benchmark::Fixture
 
 BENCHMARK_DEFINE_F(TracezProcessor, BM_RunComplete)(benchmark::State &state)
 {
+  const int numSpans = state.range(0);
   for (auto _ : state)
   {
     StartManySpans(spans, tracer, numSpans);
@@ -81,6 +80,7 @@ BENCHMARK_DEFINE_F(TracezProcessor, BM_RunComplete)(benchmark::State &state)
  */
 BENCHMARK_DEFINE_F(TracezProcessor, BM_RunSnap)(benchmark::State &state)
 {
+  const int numSpans = state.range(0);
   for (auto _ : state)
   {
     std::thread start(StartManySpans, std::ref(spans), tracer, numSpans);
@@ -98,6 +98,7 @@ BENCHMARK_DEFINE_F(TracezProcessor, BM_RunSnap)(benchmark::State &state)
  */
 BENCHMARK_DEFINE_F(TracezProcessor, BM_SnapComplete)(benchmark::State &state)
 {
+  const int numSpans = state.range(0);
   for (auto _ : state)
   {
     StartManySpans(spans, tracer, numSpans);
@@ -116,6 +117,7 @@ BENCHMARK_DEFINE_F(TracezProcessor, BM_SnapComplete)(benchmark::State &state)
  */
 BENCHMARK_DEFINE_F(TracezProcessor, BM_RunSnapComplete)(benchmark::State &state)
 {
+  const int numSpans = state.range(0);
   for (auto _ : state)
   {
     std::vector<opentelemetry::nostd::shared_ptr<opentelemetry::trace::Span>> spans2;

From 99a8bcf54ea20e13fea6b385c155ef1821e4481d Mon Sep 17 00:00:00 2001
From: Janet Vu <vjanet@umich.edu>
Date: Sun, 23 Aug 2020 05:46:31 +0000
Subject: [PATCH 11/40] Update threading

---
 .../test/tracez_data_aggregator_benchmark.cc  | 12 +++++++----
 ext/zpages/test/tracez_processor_benchmark.cc | 21 ++++++++++++-------
 2 files changed, 21 insertions(+), 12 deletions(-)

diff --git a/ext/zpages/test/tracez_data_aggregator_benchmark.cc b/ext/zpages/test/tracez_data_aggregator_benchmark.cc
index b3f8bda97c..6db7b55912 100644
--- a/ext/zpages/test/tracez_data_aggregator_benchmark.cc
+++ b/ext/zpages/test/tracez_data_aggregator_benchmark.cc
@@ -127,9 +127,11 @@ BENCHMARK_DEFINE_F(TracezAggregator, BM_BucketsSame)(benchmark::State &state)
   for (auto _ : state)
   {
     std::vector<opentelemetry::nostd::shared_ptr<opentelemetry::trace::Span>> running_spans;
-    StartSpans(running_spans, tracer, numSpans / 3);
-    StartEndSpansError(tracer, numSpans / 3);
+    std::thread run(StartSpans, std::ref(running_spans), tracer, numSpans / 3, false);
+    std::thread err(StartEndSpansError, tracer, numSpans / 3, false);
     StartEndSpansLatency(tracer, numSpans / 3);
+    run.join();
+    err.join();
   }
 }
 
@@ -143,9 +145,11 @@ BENCHMARK_DEFINE_F(TracezAggregator, BM_BucketsUnique)(benchmark::State &state)
   for (auto _ : state)
   {
     std::vector<opentelemetry::nostd::shared_ptr<opentelemetry::trace::Span>> running_spans;
-    StartSpans(running_spans, tracer, numSpans / 3, true);
-    StartEndSpansError(tracer, numSpans / 3, true);
+    std::thread run(StartSpans, std::ref(running_spans), tracer, numSpans / 3, true);
+    std::thread err(StartEndSpansError, tracer, numSpans / 3, true);
     StartEndSpansLatency(tracer, numSpans / 3, true);
+    run.join();
+    err.join();
   }
 }
 /////////////////////// RUN BENCHMARKS ///////////////////////////
diff --git a/ext/zpages/test/tracez_processor_benchmark.cc b/ext/zpages/test/tracez_processor_benchmark.cc
index 11710213c5..86b2d823b2 100644
--- a/ext/zpages/test/tracez_processor_benchmark.cc
+++ b/ext/zpages/test/tracez_processor_benchmark.cc
@@ -70,8 +70,16 @@ BENCHMARK_DEFINE_F(TracezProcessor, BM_RunComplete)(benchmark::State &state)
   const int numSpans = state.range(0);
   for (auto _ : state)
   {
-    StartManySpans(spans, tracer, numSpans);
+    std::vector<opentelemetry::nostd::shared_ptr<opentelemetry::trace::Span>> spans2;
+    StartManySpans(spans2, tracer, numSpans);
+
+    std::thread start(StartManySpans, std::ref(spans), tracer, numSpans);
+    EndAllSpans(spans2);
+
+    start.join();
+
     EndAllSpans(spans);
+    processor->GetSpanSnapshot();
   }
 }
 
@@ -84,12 +92,11 @@ BENCHMARK_DEFINE_F(TracezProcessor, BM_RunSnap)(benchmark::State &state)
   for (auto _ : state)
   {
     std::thread start(StartManySpans, std::ref(spans), tracer, numSpans);
-    std::thread snapshots(GetManySnapshots, std::ref(processor), numSpans);
+    GetManySnapshots(processor, numSpans);
 
     start.join();
-    snapshots.join();
-
     EndAllSpans(spans);
+    processor->GetSpanSnapshot();
   }
 }
 
@@ -104,10 +111,9 @@ BENCHMARK_DEFINE_F(TracezProcessor, BM_SnapComplete)(benchmark::State &state)
     StartManySpans(spans, tracer, numSpans);
 
     std::thread snapshots(GetManySnapshots, std::ref(processor), numSpans);
-    std::thread end(EndAllSpans, std::ref(spans));
+    EndAllSpans(spans);
 
     snapshots.join();
-    end.join();
   }
 
 }
@@ -126,11 +132,10 @@ BENCHMARK_DEFINE_F(TracezProcessor, BM_RunSnapComplete)(benchmark::State &state)
 
     std::thread start(StartManySpans, std::ref(spans2), tracer, numSpans);
     std::thread snapshots(GetManySnapshots, std::ref(processor), numSpans);
-    std::thread end(EndAllSpans, std::ref(spans));
+    EndAllSpans(spans);
 
     start.join();
     snapshots.join();
-    end.join();
 
     EndAllSpans(spans2);
   }

From 6fca653ba20111b8dc3a5acc2654c2e23e04728a Mon Sep 17 00:00:00 2001
From: Janet Vu <vjanet@umich.edu>
Date: Sun, 23 Aug 2020 06:01:22 +0000
Subject: [PATCH 12/40] Add run complete snap benchmarks

---
 ext/zpages/test/tracez_processor_benchmark.cc | 61 +++++++++++++++++++
 1 file changed, 61 insertions(+)

diff --git a/ext/zpages/test/tracez_processor_benchmark.cc b/ext/zpages/test/tracez_processor_benchmark.cc
index 86b2d823b2..5fc95c781a 100644
--- a/ext/zpages/test/tracez_processor_benchmark.cc
+++ b/ext/zpages/test/tracez_processor_benchmark.cc
@@ -64,7 +64,65 @@ class TracezProcessor : public benchmark::Fixture
 /*
  * Make and end many empty spans.
  */
+BENCHMARK_DEFINE_F(TracezProcessor, BM_Run)(benchmark::State &state)
+{
+  const int numSpans = state.range(0);
+  for (auto _ : state)
+  {
+    std::vector<opentelemetry::nostd::shared_ptr<opentelemetry::trace::Span>> spans2;
+
+    std::thread start(StartManySpans, std::ref(spans), tracer, numSpans);
+    StartManySpans(spans2, tracer, numSpans);
+
+    start.join();
+
+    EndAllSpans(spans);
+    EndAllSpans(spans2);
+    processor->GetSpanSnapshot();
+  }
+}
 
+/*
+ * Make and end many empty spans.
+ */
+BENCHMARK_DEFINE_F(TracezProcessor, BM_Complete)(benchmark::State &state)
+{
+  const int numSpans = state.range(0);
+  for (auto _ : state)
+  {
+    std::vector<opentelemetry::nostd::shared_ptr<opentelemetry::trace::Span>> spans2;
+    StartManySpans(spans, tracer, numSpans);
+    StartManySpans(spans2, tracer, numSpans);
+
+    std::thread end(EndAllSpans, std::ref(spans));
+    EndAllSpans(spans2);
+
+    end.join();
+    processor->GetSpanSnapshot();
+  }
+}
+
+/*
+ * Make and end many empty spans.
+ */
+BENCHMARK_DEFINE_F(TracezProcessor, BM_Snap)(benchmark::State &state)
+{
+  const int numSpans = state.range(0);
+  for (auto _ : state)
+  {
+    StartManySpans(spans, tracer, numSpans);
+
+    std::thread snap(GetManySnapshots, std::ref(processor), numSpans);
+    GetManySnapshots(processor, numSpans);
+
+    snap.join();
+    EndAllSpans(spans);
+  }
+}
+
+/*
+ * Make and end many empty spans.
+ */
 BENCHMARK_DEFINE_F(TracezProcessor, BM_RunComplete)(benchmark::State &state)
 {
   const int numSpans = state.range(0);
@@ -143,6 +201,9 @@ BENCHMARK_DEFINE_F(TracezProcessor, BM_RunSnapComplete)(benchmark::State &state)
 
 /////////////////////// RUN BENCHMARKS ///////////////////////////
 
+BENCHMARK_REGISTER_F(TracezProcessor, BM_Run)->Arg(10)->Arg(1000);
+BENCHMARK_REGISTER_F(TracezProcessor, BM_Complete)->Arg(10)->Arg(1000);
+BENCHMARK_REGISTER_F(TracezProcessor, BM_Snap)->Arg(10)->Arg(1000);
 BENCHMARK_REGISTER_F(TracezProcessor, BM_RunComplete)->Arg(10)->Arg(1000);
 BENCHMARK_REGISTER_F(TracezProcessor, BM_RunSnap)->Arg(10)->Arg(1000);
 BENCHMARK_REGISTER_F(TracezProcessor, BM_SnapComplete)->Arg(10)->Arg(1000);

From be60db8426cab7c5271b15f79a01d857b40faf45 Mon Sep 17 00:00:00 2001
From: Janet Vu <vjanet@umich.edu>
Date: Sun, 23 Aug 2020 19:42:47 +0000
Subject: [PATCH 13/40] Remove unneeded code

---
 ext/zpages/test/tracez_processor_benchmark.cc | 44 ++-----------------
 1 file changed, 3 insertions(+), 41 deletions(-)

diff --git a/ext/zpages/test/tracez_processor_benchmark.cc b/ext/zpages/test/tracez_processor_benchmark.cc
index 5fc95c781a..085d942433 100644
--- a/ext/zpages/test/tracez_processor_benchmark.cc
+++ b/ext/zpages/test/tracez_processor_benchmark.cc
@@ -69,36 +69,7 @@ BENCHMARK_DEFINE_F(TracezProcessor, BM_Run)(benchmark::State &state)
   const int numSpans = state.range(0);
   for (auto _ : state)
   {
-    std::vector<opentelemetry::nostd::shared_ptr<opentelemetry::trace::Span>> spans2;
-
-    std::thread start(StartManySpans, std::ref(spans), tracer, numSpans);
-    StartManySpans(spans2, tracer, numSpans);
-
-    start.join();
-
-    EndAllSpans(spans);
-    EndAllSpans(spans2);
-    processor->GetSpanSnapshot();
-  }
-}
-
-/*
- * Make and end many empty spans.
- */
-BENCHMARK_DEFINE_F(TracezProcessor, BM_Complete)(benchmark::State &state)
-{
-  const int numSpans = state.range(0);
-  for (auto _ : state)
-  {
-    std::vector<opentelemetry::nostd::shared_ptr<opentelemetry::trace::Span>> spans2;
     StartManySpans(spans, tracer, numSpans);
-    StartManySpans(spans2, tracer, numSpans);
-
-    std::thread end(EndAllSpans, std::ref(spans));
-    EndAllSpans(spans2);
-
-    end.join();
-    processor->GetSpanSnapshot();
   }
 }
 
@@ -110,13 +81,7 @@ BENCHMARK_DEFINE_F(TracezProcessor, BM_Snap)(benchmark::State &state)
   const int numSpans = state.range(0);
   for (auto _ : state)
   {
-    StartManySpans(spans, tracer, numSpans);
-
-    std::thread snap(GetManySnapshots, std::ref(processor), numSpans);
     GetManySnapshots(processor, numSpans);
-
-    snap.join();
-    EndAllSpans(spans);
   }
 }
 
@@ -137,7 +102,6 @@ BENCHMARK_DEFINE_F(TracezProcessor, BM_RunComplete)(benchmark::State &state)
     start.join();
 
     EndAllSpans(spans);
-    processor->GetSpanSnapshot();
   }
 }
 
@@ -149,12 +113,11 @@ BENCHMARK_DEFINE_F(TracezProcessor, BM_RunSnap)(benchmark::State &state)
   const int numSpans = state.range(0);
   for (auto _ : state)
   {
-    std::thread start(StartManySpans, std::ref(spans), tracer, numSpans);
-    GetManySnapshots(processor, numSpans);
+    std::thread snapshots(GetManySnapshots, std::ref(processor), numSpans);
+    StartManySpans(spans, tracer, numSpans);
 
-    start.join();
+    snapshots.join();
     EndAllSpans(spans);
-    processor->GetSpanSnapshot();
   }
 }
 
@@ -202,7 +165,6 @@ BENCHMARK_DEFINE_F(TracezProcessor, BM_RunSnapComplete)(benchmark::State &state)
 /////////////////////// RUN BENCHMARKS ///////////////////////////
 
 BENCHMARK_REGISTER_F(TracezProcessor, BM_Run)->Arg(10)->Arg(1000);
-BENCHMARK_REGISTER_F(TracezProcessor, BM_Complete)->Arg(10)->Arg(1000);
 BENCHMARK_REGISTER_F(TracezProcessor, BM_Snap)->Arg(10)->Arg(1000);
 BENCHMARK_REGISTER_F(TracezProcessor, BM_RunComplete)->Arg(10)->Arg(1000);
 BENCHMARK_REGISTER_F(TracezProcessor, BM_RunSnap)->Arg(10)->Arg(1000);

From e1f36c78f36f22c69d23ab6c1829e9293075bef9 Mon Sep 17 00:00:00 2001
From: Janet Vu <vjanet@umich.edu>
Date: Sun, 23 Aug 2020 19:44:26 +0000
Subject: [PATCH 14/40] Update code comments

---
 ext/zpages/test/tracez_processor_benchmark.cc | 7 +++----
 1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/ext/zpages/test/tracez_processor_benchmark.cc b/ext/zpages/test/tracez_processor_benchmark.cc
index 085d942433..c0d82d1eca 100644
--- a/ext/zpages/test/tracez_processor_benchmark.cc
+++ b/ext/zpages/test/tracez_processor_benchmark.cc
@@ -12,8 +12,7 @@ using namespace opentelemetry::ext::zpages;
 /////////////////////////////// BENCHMARK HELPER FUNCTIONS //////////////////////////////
 
 /*
- * Helper function that creates i spans, which are added into the passed
- * in vector. Used for testing thread safety
+ * Helper function that creates and add i spans into the passed in vector
  */
 void StartManySpans(
     std::vector<opentelemetry::nostd::shared_ptr<opentelemetry::trace::Span>> &spans,
@@ -62,7 +61,7 @@ class TracezProcessor : public benchmark::Fixture
 //////////////////////////// BENCHMARK DEFINITIONS /////////////////////////////////
 
 /*
- * Make and end many empty spans.
+ * Make many empty spans.
  */
 BENCHMARK_DEFINE_F(TracezProcessor, BM_Run)(benchmark::State &state)
 {
@@ -74,7 +73,7 @@ BENCHMARK_DEFINE_F(TracezProcessor, BM_Run)(benchmark::State &state)
 }
 
 /*
- * Make and end many empty spans.
+ * Make many snapshots.
  */
 BENCHMARK_DEFINE_F(TracezProcessor, BM_Snap)(benchmark::State &state)
 {

From 00d31a1425275d07665c33a913dbb50ccfbfee7e Mon Sep 17 00:00:00 2001
From: Janet Vu <vjanet@umich.edu>
Date: Mon, 24 Aug 2020 14:28:56 +0000
Subject: [PATCH 15/40] Pass tracer by ref, test getting aggregations too

---
 .../test/tracez_data_aggregator_benchmark.cc  | 94 ++++++++++++++++---
 ext/zpages/test/tracez_processor_benchmark.cc | 16 ++--
 2 files changed, 87 insertions(+), 23 deletions(-)

diff --git a/ext/zpages/test/tracez_data_aggregator_benchmark.cc b/ext/zpages/test/tracez_data_aggregator_benchmark.cc
index 6db7b55912..75b7b01edf 100644
--- a/ext/zpages/test/tracez_data_aggregator_benchmark.cc
+++ b/ext/zpages/test/tracez_data_aggregator_benchmark.cc
@@ -16,7 +16,7 @@ using opentelemetry::core::SteadyTimestamp;
  * Helper function that creates and ends i spans instantly
  */
 void StartEndSpans(
-    std::shared_ptr<opentelemetry::trace::Tracer> tracer,
+    std::shared_ptr<opentelemetry::trace::Tracer> &tracer,
     int i,
     bool isUnique = false)
 {
@@ -29,7 +29,7 @@ void StartEndSpans(
  * different latencies.
  */
 void StartEndSpansLatency(
-    std::shared_ptr<opentelemetry::trace::Tracer> tracer,
+    std::shared_ptr<opentelemetry::trace::Tracer> &tracer,
     int i,
     bool isUnique = false)
 {
@@ -50,7 +50,7 @@ void StartEndSpansLatency(
  * error codes.
  */
 void StartEndSpansError(
-    std::shared_ptr<opentelemetry::trace::Tracer> tracer,
+    std::shared_ptr<opentelemetry::trace::Tracer> &tracer,
     int i,
     bool isUnique = false)
 {
@@ -64,7 +64,7 @@ void StartEndSpansError(
  */
 void StartSpans(
     std::vector<opentelemetry::nostd::shared_ptr<opentelemetry::trace::Span>> &spans,
-    std::shared_ptr<opentelemetry::trace::Tracer> tracer,
+    std::shared_ptr<opentelemetry::trace::Tracer> &tracer,
     int i,
     bool isUnique = false)
 {
@@ -72,6 +72,12 @@ void StartSpans(
     spans.push_back(tracer->StartSpan(isUnique ? std::to_string(i) : ""));
 }
 
+void GetManyAggregations(std::unique_ptr<TracezDataAggregator> &aggregator, int i)
+{
+  for (; i > 0; i--)
+    aggregator->GetAggregatedTracezData();
+}
+
 ////////////////////////  FIXTURE FOR SHARED SETUP CODE ///////////////////
 
 class TracezAggregator : public benchmark::Fixture
@@ -80,21 +86,22 @@ class TracezAggregator : public benchmark::Fixture
   void SetUp(const ::benchmark::State& state)
   {
     std::shared_ptr<TracezSpanProcessor> processor(new TracezSpanProcessor());
-    tracer                 = std::shared_ptr<opentelemetry::trace::Tracer>(new Tracer(processor));
-    tracez_data_aggregator = std::unique_ptr<TracezDataAggregator>(
+    tracer     = std::shared_ptr<opentelemetry::trace::Tracer>(new Tracer(processor));
+    aggregator = std::unique_ptr<TracezDataAggregator>(
         new TracezDataAggregator(processor, milliseconds(10)));
   }
 
-  std::unique_ptr<TracezDataAggregator> tracez_data_aggregator;
+  std::unique_ptr<TracezDataAggregator> aggregator;
   std::shared_ptr<opentelemetry::trace::Tracer> tracer;
 };
 
 //////////////////////////// BENCHMARK DEFINITIONS /////////////////////////////////
 
+////////////// BENCHMARKS WHERE USER NEVER VISITS WEBPAGE /////////////////////////
+
 /*
  * Aggregator handing many spans with the same name, who end instantly.
  */
-
 BENCHMARK_DEFINE_F(TracezAggregator, BM_InstantSame)(benchmark::State &state)
 {
   const int numSpans = state.range(0);
@@ -107,7 +114,6 @@ BENCHMARK_DEFINE_F(TracezAggregator, BM_InstantSame)(benchmark::State &state)
 /*
  * Aggregator handing many spans with unique names, who end instantly.
  */
-
 BENCHMARK_DEFINE_F(TracezAggregator, BM_InstantUnique)(benchmark::State &state)
 {
   const int numSpans = state.range(0);
@@ -120,15 +126,14 @@ BENCHMARK_DEFINE_F(TracezAggregator, BM_InstantUnique)(benchmark::State &state)
 /*
  * Aggregator handing many spans with the same name, who end instantly.
  */
-
 BENCHMARK_DEFINE_F(TracezAggregator, BM_BucketsSame)(benchmark::State &state)
 {
   const int numSpans = state.range(0);
   for (auto _ : state)
   {
     std::vector<opentelemetry::nostd::shared_ptr<opentelemetry::trace::Span>> running_spans;
-    std::thread run(StartSpans, std::ref(running_spans), tracer, numSpans / 3, false);
-    std::thread err(StartEndSpansError, tracer, numSpans / 3, false);
+    std::thread run(StartSpans, std::ref(running_spans), std::ref(tracer), numSpans / 3, false);
+    std::thread err(StartEndSpansError, std::ref(tracer), numSpans / 3, false);
     StartEndSpansLatency(tracer, numSpans / 3);
     run.join();
     err.join();
@@ -138,26 +143,85 @@ BENCHMARK_DEFINE_F(TracezAggregator, BM_BucketsSame)(benchmark::State &state)
 /*
  * Aggregator handing many spans with unique names, who end instantly.
  */
-
 BENCHMARK_DEFINE_F(TracezAggregator, BM_BucketsUnique)(benchmark::State &state)
 {
   const int numSpans = state.range(0);
   for (auto _ : state)
   {
     std::vector<opentelemetry::nostd::shared_ptr<opentelemetry::trace::Span>> running_spans;
-    std::thread run(StartSpans, std::ref(running_spans), tracer, numSpans / 3, true);
-    std::thread err(StartEndSpansError, tracer, numSpans / 3, true);
+    std::thread run(StartSpans, std::ref(running_spans), std::ref(tracer), numSpans / 3, true);
+    std::thread err(StartEndSpansError, std::ref(tracer), numSpans / 3, true);
     StartEndSpansLatency(tracer, numSpans / 3, true);
     run.join();
     err.join();
   }
 }
+
+////////////// SAME BENCHMARKS, BUT USER VISITS WEBPAGE /////////////////////////
+
+BENCHMARK_DEFINE_F(TracezAggregator, BM_InstantSameGet)(benchmark::State &state)
+{
+  const int numSpans = state.range(0);
+  for (auto _ : state)
+  {
+    std::thread spans(StartEndSpans, std::ref(tracer), numSpans, false);
+    GetManyAggregations(std::ref(aggregator), numSpans);
+    spans.join();
+  }
+}
+
+BENCHMARK_DEFINE_F(TracezAggregator, BM_InstantUniqueGet)(benchmark::State &state)
+{
+  const int numSpans = state.range(0);
+  for (auto _ : state)
+  {
+    std::thread spans(StartEndSpans, std::ref(tracer), numSpans, true);
+    GetManyAggregations(std::ref(aggregator), numSpans);
+    spans.join();
+  }
+}
+
+BENCHMARK_DEFINE_F(TracezAggregator, BM_BucketsSameGet)(benchmark::State &state)
+{
+  const int numSpans = state.range(0);
+  for (auto _ : state)
+  {
+    std::vector<opentelemetry::nostd::shared_ptr<opentelemetry::trace::Span>> running_spans;
+    std::thread run(StartSpans, std::ref(running_spans), std::ref(tracer), numSpans / 3, false);
+    std::thread err(StartEndSpansError, std::ref(tracer), numSpans / 3, false);
+    std::thread lat(StartEndSpansLatency, std::ref(tracer), numSpans / 3, false);
+    GetManyAggregations(std::ref(aggregator), numSpans / 3);
+    run.join();
+    err.join();
+    lat.join();
+  }
+}
+
+BENCHMARK_DEFINE_F(TracezAggregator, BM_BucketsUniqueGet)(benchmark::State &state)
+{
+  const int numSpans = state.range(0);
+  for (auto _ : state)
+  {
+    std::vector<opentelemetry::nostd::shared_ptr<opentelemetry::trace::Span>> running_spans;
+    std::thread run(StartSpans, std::ref(running_spans), std::ref(tracer), numSpans / 3, true);
+    std::thread err(StartEndSpansError, std::ref(tracer), numSpans / 3, true);
+    std::thread lat(StartEndSpansLatency, std::ref(tracer), numSpans / 3, false);
+    GetManyAggregations(std::ref(aggregator), numSpans / 3);
+    run.join();
+    err.join();
+    lat.join();
+  }
+}
 /////////////////////// RUN BENCHMARKS ///////////////////////////
 
 BENCHMARK_REGISTER_F(TracezAggregator, BM_InstantSame)->Arg(10)->Arg(1000);
 BENCHMARK_REGISTER_F(TracezAggregator, BM_InstantUnique)->Arg(10)->Arg(1000);
 BENCHMARK_REGISTER_F(TracezAggregator, BM_BucketsSame)->Arg(10)->Arg(1000);
 BENCHMARK_REGISTER_F(TracezAggregator, BM_BucketsUnique)->Arg(10)->Arg(1000);
+BENCHMARK_REGISTER_F(TracezAggregator, BM_InstantSameGet)->Arg(10)->Arg(1000);
+BENCHMARK_REGISTER_F(TracezAggregator, BM_InstantUniqueGet)->Arg(10)->Arg(1000);
+BENCHMARK_REGISTER_F(TracezAggregator, BM_BucketsSameGet)->Arg(10)->Arg(1000);
+BENCHMARK_REGISTER_F(TracezAggregator, BM_BucketsUniqueGet)->Arg(10)->Arg(1000);
 
 BENCHMARK_MAIN();
 
diff --git a/ext/zpages/test/tracez_processor_benchmark.cc b/ext/zpages/test/tracez_processor_benchmark.cc
index c0d82d1eca..7eb12e92bb 100644
--- a/ext/zpages/test/tracez_processor_benchmark.cc
+++ b/ext/zpages/test/tracez_processor_benchmark.cc
@@ -16,7 +16,7 @@ using namespace opentelemetry::ext::zpages;
  */
 void StartManySpans(
     std::vector<opentelemetry::nostd::shared_ptr<opentelemetry::trace::Span>> &spans,
-    std::shared_ptr<opentelemetry::trace::Tracer> tracer,
+    std::shared_ptr<opentelemetry::trace::Tracer> &tracer,
     int i)
 {
   for (; i > 0; i--)
@@ -68,7 +68,7 @@ BENCHMARK_DEFINE_F(TracezProcessor, BM_Run)(benchmark::State &state)
   const int numSpans = state.range(0);
   for (auto _ : state)
   {
-    StartManySpans(spans, tracer, numSpans);
+    StartManySpans(spans, std::ref(tracer), numSpans);
   }
 }
 
@@ -93,9 +93,9 @@ BENCHMARK_DEFINE_F(TracezProcessor, BM_RunComplete)(benchmark::State &state)
   for (auto _ : state)
   {
     std::vector<opentelemetry::nostd::shared_ptr<opentelemetry::trace::Span>> spans2;
-    StartManySpans(spans2, tracer, numSpans);
+    StartManySpans(spans2, std::ref(tracer), numSpans);
 
-    std::thread start(StartManySpans, std::ref(spans), tracer, numSpans);
+    std::thread start(StartManySpans, std::ref(spans), std::ref(tracer), numSpans);
     EndAllSpans(spans2);
 
     start.join();
@@ -113,7 +113,7 @@ BENCHMARK_DEFINE_F(TracezProcessor, BM_RunSnap)(benchmark::State &state)
   for (auto _ : state)
   {
     std::thread snapshots(GetManySnapshots, std::ref(processor), numSpans);
-    StartManySpans(spans, tracer, numSpans);
+    StartManySpans(spans, std::ref(tracer), numSpans);
 
     snapshots.join();
     EndAllSpans(spans);
@@ -128,7 +128,7 @@ BENCHMARK_DEFINE_F(TracezProcessor, BM_SnapComplete)(benchmark::State &state)
   const int numSpans = state.range(0);
   for (auto _ : state)
   {
-    StartManySpans(spans, tracer, numSpans);
+    StartManySpans(spans, std::ref(tracer), numSpans);
 
     std::thread snapshots(GetManySnapshots, std::ref(processor), numSpans);
     EndAllSpans(spans);
@@ -148,9 +148,9 @@ BENCHMARK_DEFINE_F(TracezProcessor, BM_RunSnapComplete)(benchmark::State &state)
   {
     std::vector<opentelemetry::nostd::shared_ptr<opentelemetry::trace::Span>> spans2;
 
-    StartManySpans(spans, tracer, numSpans);
+    StartManySpans(spans, std::ref(tracer), numSpans);
 
-    std::thread start(StartManySpans, std::ref(spans2), tracer, numSpans);
+    std::thread start(StartManySpans, std::ref(spans2), std::ref(tracer), numSpans);
     std::thread snapshots(GetManySnapshots, std::ref(processor), numSpans);
     EndAllSpans(spans);
 

From 0da246d8066f7d11b30db89ff68591fab9ee810f Mon Sep 17 00:00:00 2001
From: Janet Vu <vjanet@umich.edu>
Date: Mon, 24 Aug 2020 14:55:15 +0000
Subject: [PATCH 16/40] Add more descriptive comments

---
 .../test/tracez_data_aggregator_benchmark.cc  | 28 ++++++++++++++-----
 ext/zpages/test/tracez_processor_benchmark.cc | 24 +++++++++++-----
 2 files changed, 38 insertions(+), 14 deletions(-)

diff --git a/ext/zpages/test/tracez_data_aggregator_benchmark.cc b/ext/zpages/test/tracez_data_aggregator_benchmark.cc
index 75b7b01edf..ec3413a5e1 100644
--- a/ext/zpages/test/tracez_data_aggregator_benchmark.cc
+++ b/ext/zpages/test/tracez_data_aggregator_benchmark.cc
@@ -100,7 +100,10 @@ class TracezAggregator : public benchmark::Fixture
 ////////////// BENCHMARKS WHERE USER NEVER VISITS WEBPAGE /////////////////////////
 
 /*
- * Aggregator handing many spans with the same name, who end instantly.
+ * Aggregator handing many spans with the same name, who end instantly. This
+ * checks the scenario where there's only one Tracez name and minimal sorting
+ * of latencies is required, as all spans should be sorted in the same bucket
+ * under the same span name.
  */
 BENCHMARK_DEFINE_F(TracezAggregator, BM_InstantSame)(benchmark::State &state)
 {
@@ -112,7 +115,10 @@ BENCHMARK_DEFINE_F(TracezAggregator, BM_InstantSame)(benchmark::State &state)
 }
 
 /*
- * Aggregator handing many spans with unique names, who end instantly.
+ * Aggregator handing many spans with unique names, who end instantly. This
+ * checks the scenario where there's many Tracez groups but minimal sorting
+ * of latencies is required. Spans are sorted in different groups but always
+ * in the same bucket.
  */
 BENCHMARK_DEFINE_F(TracezAggregator, BM_InstantUnique)(benchmark::State &state)
 {
@@ -124,7 +130,11 @@ BENCHMARK_DEFINE_F(TracezAggregator, BM_InstantUnique)(benchmark::State &state)
 }
 
 /*
- * Aggregator handing many spans with the same name, who end instantly.
+ * Aggregator handing many spans with the same name, who may fall under error,
+ * running, and any latency group. This checks the scenario where there's only
+ * one Tracez groups but there also needs to be sorting of spans into their
+ * respective buckets. Spans are in the same group but sorted to different
+ * buckets.
  */
 BENCHMARK_DEFINE_F(TracezAggregator, BM_BucketsSame)(benchmark::State &state)
 {
@@ -141,7 +151,11 @@ BENCHMARK_DEFINE_F(TracezAggregator, BM_BucketsSame)(benchmark::State &state)
 }
 
 /*
- * Aggregator handing many spans with unique names, who end instantly.
+ * Aggregator handing many spans with unique names, who may fall under error,
+ * running, and any latency group. This checks the scenario where there's many
+ * Tracez groups and there needs to be sorting of spans into their
+ * respective buckets. Spans are in the different groups and buckets, similar to
+ * real likely use cases.
  */
 BENCHMARK_DEFINE_F(TracezAggregator, BM_BucketsUnique)(benchmark::State &state)
 {
@@ -215,12 +229,12 @@ BENCHMARK_DEFINE_F(TracezAggregator, BM_BucketsUniqueGet)(benchmark::State &stat
 /////////////////////// RUN BENCHMARKS ///////////////////////////
 
 BENCHMARK_REGISTER_F(TracezAggregator, BM_InstantSame)->Arg(10)->Arg(1000);
-BENCHMARK_REGISTER_F(TracezAggregator, BM_InstantUnique)->Arg(10)->Arg(1000);
-BENCHMARK_REGISTER_F(TracezAggregator, BM_BucketsSame)->Arg(10)->Arg(1000);
-BENCHMARK_REGISTER_F(TracezAggregator, BM_BucketsUnique)->Arg(10)->Arg(1000);
 BENCHMARK_REGISTER_F(TracezAggregator, BM_InstantSameGet)->Arg(10)->Arg(1000);
+BENCHMARK_REGISTER_F(TracezAggregator, BM_InstantUnique)->Arg(10)->Arg(1000);
 BENCHMARK_REGISTER_F(TracezAggregator, BM_InstantUniqueGet)->Arg(10)->Arg(1000);
+BENCHMARK_REGISTER_F(TracezAggregator, BM_BucketsSame)->Arg(10)->Arg(1000);
 BENCHMARK_REGISTER_F(TracezAggregator, BM_BucketsSameGet)->Arg(10)->Arg(1000);
+BENCHMARK_REGISTER_F(TracezAggregator, BM_BucketsUnique)->Arg(10)->Arg(1000);
 BENCHMARK_REGISTER_F(TracezAggregator, BM_BucketsUniqueGet)->Arg(10)->Arg(1000);
 
 BENCHMARK_MAIN();
diff --git a/ext/zpages/test/tracez_processor_benchmark.cc b/ext/zpages/test/tracez_processor_benchmark.cc
index 7eb12e92bb..e2895674ee 100644
--- a/ext/zpages/test/tracez_processor_benchmark.cc
+++ b/ext/zpages/test/tracez_processor_benchmark.cc
@@ -33,7 +33,9 @@ void EndAllSpans(std::vector<opentelemetry::nostd::shared_ptr<opentelemetry::tra
 }
 
 /*
- * Helper function calls GetSpanSnapshot() i times, does nothing otherwise
+ * Helper function calls GetSpanSnapshot() i times, does nothing otherwise.
+ * Snapshots are significant and contribute to performance differences because
+ * completed spans are cleared from the processor memory.
  */
 void GetManySnapshots(std::shared_ptr<TracezSpanProcessor> &processor, int i)
 {
@@ -61,7 +63,8 @@ class TracezProcessor : public benchmark::Fixture
 //////////////////////////// BENCHMARK DEFINITIONS /////////////////////////////////
 
 /*
- * Make many empty spans.
+ * Make many empty spans. This checks the scenario where the processor holds
+ * many running spans but never gets queried.
  */
 BENCHMARK_DEFINE_F(TracezProcessor, BM_Run)(benchmark::State &state)
 {
@@ -73,7 +76,8 @@ BENCHMARK_DEFINE_F(TracezProcessor, BM_Run)(benchmark::State &state)
 }
 
 /*
- * Make many snapshots.
+ * Make many snapshots. This checks the scenario where the processor holds
+ * no spans but gets queried many times.
  */
 BENCHMARK_DEFINE_F(TracezProcessor, BM_Snap)(benchmark::State &state)
 {
@@ -85,7 +89,8 @@ BENCHMARK_DEFINE_F(TracezProcessor, BM_Snap)(benchmark::State &state)
 }
 
 /*
- * Make and end many empty spans.
+ * Make and end many empty spans. This checks the scenario where the processor holds
+ * many running and completed spans but never gets queried.
  */
 BENCHMARK_DEFINE_F(TracezProcessor, BM_RunComplete)(benchmark::State &state)
 {
@@ -105,7 +110,8 @@ BENCHMARK_DEFINE_F(TracezProcessor, BM_RunComplete)(benchmark::State &state)
 }
 
 /*
- * Make many empty spans while spapshots grabbed.
+ * Make many empty spans while spapshots grabbed. This checks the scenario where the
+ * processor holds many running spans and gets queried.
  */
 BENCHMARK_DEFINE_F(TracezProcessor, BM_RunSnap)(benchmark::State &state)
 {
@@ -121,7 +127,9 @@ BENCHMARK_DEFINE_F(TracezProcessor, BM_RunSnap)(benchmark::State &state)
 }
 
 /*
- * Make many empty spans end while snapshots are being grabbed.
+ * Make many empty spans end while snapshots are being grabbed. This checks the scenario
+ * where the processor doesn't make new spans, but existing spans complete while they're
+ * queried.
  */
 BENCHMARK_DEFINE_F(TracezProcessor, BM_SnapComplete)(benchmark::State &state)
 {
@@ -139,7 +147,9 @@ BENCHMARK_DEFINE_F(TracezProcessor, BM_SnapComplete)(benchmark::State &state)
 }
 
 /*
- * Make many empty spans and end them, all while snapshots are being grabbed.
+ * Make many empty spans and end some, all while snapshots are being grabbed. This
+ * checks the scenario where the processor makes new spans, other spans complete,
+ * and all spans are queried. This is the case most similar to real situations.
  */
 BENCHMARK_DEFINE_F(TracezProcessor, BM_RunSnapComplete)(benchmark::State &state)
 {

From 01ae6bf959be87e35918cd1abdf18073a51e22a8 Mon Sep 17 00:00:00 2001
From: Janet Vu <vjanet@umich.edu>
Date: Mon, 24 Aug 2020 14:59:18 +0000
Subject: [PATCH 17/40] Remove finished spans

---
 ext/zpages/test/tracez_processor_benchmark.cc | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/ext/zpages/test/tracez_processor_benchmark.cc b/ext/zpages/test/tracez_processor_benchmark.cc
index e2895674ee..e56d131fe3 100644
--- a/ext/zpages/test/tracez_processor_benchmark.cc
+++ b/ext/zpages/test/tracez_processor_benchmark.cc
@@ -28,8 +28,12 @@ void StartManySpans(
  */
 void EndAllSpans(std::vector<opentelemetry::nostd::shared_ptr<opentelemetry::trace::Span>> &spans)
 {
-  for (auto &span : spans)
+  while(!spans.empty())
+  {
+    auto span = spans.back();
     span->End();
+    spans.pop_back();
+  }
 }
 
 /*

From 6083ce287677d3b9819f7a188b2ee3b0bc15cd63 Mon Sep 17 00:00:00 2001
From: Janet Vu <vjanet@umich.edu>
Date: Mon, 24 Aug 2020 15:30:10 +0000
Subject: [PATCH 18/40] Fix typo, edit comments

---
 ext/zpages/test/tracez_data_aggregator_benchmark.cc | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/ext/zpages/test/tracez_data_aggregator_benchmark.cc b/ext/zpages/test/tracez_data_aggregator_benchmark.cc
index ec3413a5e1..f704bcec73 100644
--- a/ext/zpages/test/tracez_data_aggregator_benchmark.cc
+++ b/ext/zpages/test/tracez_data_aggregator_benchmark.cc
@@ -171,7 +171,7 @@ BENCHMARK_DEFINE_F(TracezAggregator, BM_BucketsUnique)(benchmark::State &state)
   }
 }
 
-////////////// SAME BENCHMARKS, BUT USER VISITS WEBPAGE /////////////////////////
+//////// SAME BENCHMARKS, BUT USER VISITS WEBPAGE--DELAYING AGGREGATION WORK /////////
 
 BENCHMARK_DEFINE_F(TracezAggregator, BM_InstantSameGet)(benchmark::State &state)
 {
@@ -219,7 +219,7 @@ BENCHMARK_DEFINE_F(TracezAggregator, BM_BucketsUniqueGet)(benchmark::State &stat
     std::vector<opentelemetry::nostd::shared_ptr<opentelemetry::trace::Span>> running_spans;
     std::thread run(StartSpans, std::ref(running_spans), std::ref(tracer), numSpans / 3, true);
     std::thread err(StartEndSpansError, std::ref(tracer), numSpans / 3, true);
-    std::thread lat(StartEndSpansLatency, std::ref(tracer), numSpans / 3, false);
+    std::thread lat(StartEndSpansLatency, std::ref(tracer), numSpans / 3, true);
     GetManyAggregations(std::ref(aggregator), numSpans / 3);
     run.join();
     err.join();

From 5c2f2955f0b80fb958aabcc4ac7503675f3c5aad Mon Sep 17 00:00:00 2001
From: Janet Vu <vjanet@umich.edu>
Date: Mon, 24 Aug 2020 15:33:36 +0000
Subject: [PATCH 19/40] Update comment

---
 ext/zpages/test/tracez_processor_benchmark.cc | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/ext/zpages/test/tracez_processor_benchmark.cc b/ext/zpages/test/tracez_processor_benchmark.cc
index e56d131fe3..68eac37f46 100644
--- a/ext/zpages/test/tracez_processor_benchmark.cc
+++ b/ext/zpages/test/tracez_processor_benchmark.cc
@@ -39,7 +39,8 @@ void EndAllSpans(std::vector<opentelemetry::nostd::shared_ptr<opentelemetry::tra
 /*
  * Helper function calls GetSpanSnapshot() i times, does nothing otherwise.
  * Snapshots are significant and contribute to performance differences because
- * completed spans are cleared from the processor memory.
+ * completed spans are cleared from the processor memory. This function
+ * simulates an aggregator querying the processor many times.
  */
 void GetManySnapshots(std::shared_ptr<TracezSpanProcessor> &processor, int i)
 {

From ad3585e6ed9e8a6fbc21d757be2f2a53ff052f0e Mon Sep 17 00:00:00 2001
From: Janet Vu <vjanet@umich.edu>
Date: Mon, 24 Aug 2020 15:36:41 +0000
Subject: [PATCH 20/40] Update comments

---
 ext/zpages/test/tracez_processor_benchmark.cc | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/ext/zpages/test/tracez_processor_benchmark.cc b/ext/zpages/test/tracez_processor_benchmark.cc
index 68eac37f46..58b3265b2a 100644
--- a/ext/zpages/test/tracez_processor_benchmark.cc
+++ b/ext/zpages/test/tracez_processor_benchmark.cc
@@ -24,7 +24,8 @@ void StartManySpans(
 }
 
 /*
- * Helper function that ends all spans in the passed in span vector.
+ * Helper function that ends and removes all spans in the passed in span vector,
+ * since they'll be in the processor memory.
  */
 void EndAllSpans(std::vector<opentelemetry::nostd::shared_ptr<opentelemetry::trace::Span>> &spans)
 {
@@ -40,7 +41,8 @@ void EndAllSpans(std::vector<opentelemetry::nostd::shared_ptr<opentelemetry::tra
  * Helper function calls GetSpanSnapshot() i times, does nothing otherwise.
  * Snapshots are significant and contribute to performance differences because
  * completed spans are cleared from the processor memory. This function
- * simulates an aggregator querying the processor many times.
+ * simulates an aggregator querying the processor many times, but doesn't
+ * process spans and throws them out.
  */
 void GetManySnapshots(std::shared_ptr<TracezSpanProcessor> &processor, int i)
 {

From aaf2567d96eaf1c27314416fd347fc7cb681d810 Mon Sep 17 00:00:00 2001
From: Janet Vu <vjanet@umich.edu>
Date: Mon, 24 Aug 2020 15:47:39 +0000
Subject: [PATCH 21/40] Grammar, clarifications

---
 .../test/tracez_data_aggregator_benchmark.cc  | 20 +++++++++++++------
 ext/zpages/test/tracez_processor_benchmark.cc |  4 ++--
 2 files changed, 16 insertions(+), 8 deletions(-)

diff --git a/ext/zpages/test/tracez_data_aggregator_benchmark.cc b/ext/zpages/test/tracez_data_aggregator_benchmark.cc
index f704bcec73..85bab9bde8 100644
--- a/ext/zpages/test/tracez_data_aggregator_benchmark.cc
+++ b/ext/zpages/test/tracez_data_aggregator_benchmark.cc
@@ -13,7 +13,7 @@ using opentelemetry::core::SteadyTimestamp;
 /////////////////////////////// BENCHMARK HELPER FUNCTIONS //////////////////////////////
 
 /*
- * Helper function that creates and ends i spans instantly
+ * Helper function that creates and ends i spans instantly.
  */
 void StartEndSpans(
     std::shared_ptr<opentelemetry::trace::Tracer> &tracer,
@@ -46,7 +46,7 @@ void StartEndSpansLatency(
   }
 }
 /*
- * Helper function that creates and ends spans instantly. while simulating
+ * Helper function that creates and ends spans instantly, while simulating
  * error codes.
  */
 void StartEndSpansError(
@@ -72,6 +72,13 @@ void StartSpans(
     spans.push_back(tracer->StartSpan(isUnique ? std::to_string(i) : ""));
 }
 
+/*
+ * Helper function that queries for Tracez data. This simulates a user visiting
+ * or refreshing the Tracez webpage many times, except this function does and
+ * holds nothing in memory. While this runs, aggregation work and performance is
+ * expected is be affected negatively.
+ */
+
 void GetManyAggregations(std::unique_ptr<TracezDataAggregator> &aggregator, int i)
 {
   for (; i > 0; i--)
@@ -97,7 +104,7 @@ class TracezAggregator : public benchmark::Fixture
 
 //////////////////////////// BENCHMARK DEFINITIONS /////////////////////////////////
 
-////////////// BENCHMARKS WHERE USER NEVER VISITS WEBPAGE /////////////////////////
+//////////////////////// USER NEVER VISITS WEBPAGE //////////////////////////////
 
 /*
  * Aggregator handing many spans with the same name, who end instantly. This
@@ -155,7 +162,7 @@ BENCHMARK_DEFINE_F(TracezAggregator, BM_BucketsSame)(benchmark::State &state)
  * running, and any latency group. This checks the scenario where there's many
  * Tracez groups and there needs to be sorting of spans into their
  * respective buckets. Spans are in the different groups and buckets, similar to
- * real likely use cases.
+ * likely real use cases.
  */
 BENCHMARK_DEFINE_F(TracezAggregator, BM_BucketsUnique)(benchmark::State &state)
 {
@@ -171,7 +178,7 @@ BENCHMARK_DEFINE_F(TracezAggregator, BM_BucketsUnique)(benchmark::State &state)
   }
 }
 
-//////// SAME BENCHMARKS, BUT USER VISITS WEBPAGE--DELAYING AGGREGATION WORK /////////
+//////////////////// SAME BENCHMARKS, BUT USER VISITS WEBPAGE ////////////////////////
 
 BENCHMARK_DEFINE_F(TracezAggregator, BM_InstantSameGet)(benchmark::State &state)
 {
@@ -226,7 +233,8 @@ BENCHMARK_DEFINE_F(TracezAggregator, BM_BucketsUniqueGet)(benchmark::State &stat
     lat.join();
   }
 }
-/////////////////////// RUN BENCHMARKS ///////////////////////////
+
+//////////////////////////// RUN BENCHMARKS ///////////////////////////////
 
 BENCHMARK_REGISTER_F(TracezAggregator, BM_InstantSame)->Arg(10)->Arg(1000);
 BENCHMARK_REGISTER_F(TracezAggregator, BM_InstantSameGet)->Arg(10)->Arg(1000);
diff --git a/ext/zpages/test/tracez_processor_benchmark.cc b/ext/zpages/test/tracez_processor_benchmark.cc
index 58b3265b2a..3781859e0a 100644
--- a/ext/zpages/test/tracez_processor_benchmark.cc
+++ b/ext/zpages/test/tracez_processor_benchmark.cc
@@ -12,7 +12,7 @@ using namespace opentelemetry::ext::zpages;
 /////////////////////////////// BENCHMARK HELPER FUNCTIONS //////////////////////////////
 
 /*
- * Helper function that creates and add i spans into the passed in vector
+ * Helper function that creates and add i spans into the passed in vector.
  */
 void StartManySpans(
     std::vector<opentelemetry::nostd::shared_ptr<opentelemetry::trace::Span>> &spans,
@@ -42,7 +42,7 @@ void EndAllSpans(std::vector<opentelemetry::nostd::shared_ptr<opentelemetry::tra
  * Snapshots are significant and contribute to performance differences because
  * completed spans are cleared from the processor memory. This function
  * simulates an aggregator querying the processor many times, but doesn't
- * process spans and throws them out.
+ * aggregate or store spans.
  */
 void GetManySnapshots(std::shared_ptr<TracezSpanProcessor> &processor, int i)
 {

From 3d2a960570320f65bac60a6f47f332309aad2e4e Mon Sep 17 00:00:00 2001
From: Janet Vu <vjanet@umich.edu>
Date: Mon, 24 Aug 2020 15:49:40 +0000
Subject: [PATCH 22/40] numSpans->num_spans

---
 .../test/tracez_data_aggregator_benchmark.cc  | 56 +++++++++----------
 ext/zpages/test/tracez_processor_benchmark.cc | 34 +++++------
 2 files changed, 45 insertions(+), 45 deletions(-)

diff --git a/ext/zpages/test/tracez_data_aggregator_benchmark.cc b/ext/zpages/test/tracez_data_aggregator_benchmark.cc
index 85bab9bde8..e69f915b49 100644
--- a/ext/zpages/test/tracez_data_aggregator_benchmark.cc
+++ b/ext/zpages/test/tracez_data_aggregator_benchmark.cc
@@ -114,10 +114,10 @@ class TracezAggregator : public benchmark::Fixture
  */
 BENCHMARK_DEFINE_F(TracezAggregator, BM_InstantSame)(benchmark::State &state)
 {
-  const int numSpans = state.range(0);
+  const int num_spans = state.range(0);
   for (auto _ : state)
   {
-    StartEndSpans(tracer, numSpans);
+    StartEndSpans(tracer, num_spans);
   }
 }
 
@@ -129,10 +129,10 @@ BENCHMARK_DEFINE_F(TracezAggregator, BM_InstantSame)(benchmark::State &state)
  */
 BENCHMARK_DEFINE_F(TracezAggregator, BM_InstantUnique)(benchmark::State &state)
 {
-  const int numSpans = state.range(0);
+  const int num_spans = state.range(0);
   for (auto _ : state)
   {
-    StartEndSpans(tracer, numSpans, true);
+    StartEndSpans(tracer, num_spans, true);
   }
 }
 
@@ -145,13 +145,13 @@ BENCHMARK_DEFINE_F(TracezAggregator, BM_InstantUnique)(benchmark::State &state)
  */
 BENCHMARK_DEFINE_F(TracezAggregator, BM_BucketsSame)(benchmark::State &state)
 {
-  const int numSpans = state.range(0);
+  const int num_spans = state.range(0);
   for (auto _ : state)
   {
     std::vector<opentelemetry::nostd::shared_ptr<opentelemetry::trace::Span>> running_spans;
-    std::thread run(StartSpans, std::ref(running_spans), std::ref(tracer), numSpans / 3, false);
-    std::thread err(StartEndSpansError, std::ref(tracer), numSpans / 3, false);
-    StartEndSpansLatency(tracer, numSpans / 3);
+    std::thread run(StartSpans, std::ref(running_spans), std::ref(tracer), num_spans / 3, false);
+    std::thread err(StartEndSpansError, std::ref(tracer), num_spans / 3, false);
+    StartEndSpansLatency(tracer, num_spans / 3);
     run.join();
     err.join();
   }
@@ -166,13 +166,13 @@ BENCHMARK_DEFINE_F(TracezAggregator, BM_BucketsSame)(benchmark::State &state)
  */
 BENCHMARK_DEFINE_F(TracezAggregator, BM_BucketsUnique)(benchmark::State &state)
 {
-  const int numSpans = state.range(0);
+  const int num_spans = state.range(0);
   for (auto _ : state)
   {
     std::vector<opentelemetry::nostd::shared_ptr<opentelemetry::trace::Span>> running_spans;
-    std::thread run(StartSpans, std::ref(running_spans), std::ref(tracer), numSpans / 3, true);
-    std::thread err(StartEndSpansError, std::ref(tracer), numSpans / 3, true);
-    StartEndSpansLatency(tracer, numSpans / 3, true);
+    std::thread run(StartSpans, std::ref(running_spans), std::ref(tracer), num_spans / 3, true);
+    std::thread err(StartEndSpansError, std::ref(tracer), num_spans / 3, true);
+    StartEndSpansLatency(tracer, num_spans / 3, true);
     run.join();
     err.join();
   }
@@ -182,36 +182,36 @@ BENCHMARK_DEFINE_F(TracezAggregator, BM_BucketsUnique)(benchmark::State &state)
 
 BENCHMARK_DEFINE_F(TracezAggregator, BM_InstantSameGet)(benchmark::State &state)
 {
-  const int numSpans = state.range(0);
+  const int num_spans = state.range(0);
   for (auto _ : state)
   {
-    std::thread spans(StartEndSpans, std::ref(tracer), numSpans, false);
-    GetManyAggregations(std::ref(aggregator), numSpans);
+    std::thread spans(StartEndSpans, std::ref(tracer), num_spans, false);
+    GetManyAggregations(std::ref(aggregator), num_spans);
     spans.join();
   }
 }
 
 BENCHMARK_DEFINE_F(TracezAggregator, BM_InstantUniqueGet)(benchmark::State &state)
 {
-  const int numSpans = state.range(0);
+  const int num_spans = state.range(0);
   for (auto _ : state)
   {
-    std::thread spans(StartEndSpans, std::ref(tracer), numSpans, true);
-    GetManyAggregations(std::ref(aggregator), numSpans);
+    std::thread spans(StartEndSpans, std::ref(tracer), num_spans, true);
+    GetManyAggregations(std::ref(aggregator), num_spans);
     spans.join();
   }
 }
 
 BENCHMARK_DEFINE_F(TracezAggregator, BM_BucketsSameGet)(benchmark::State &state)
 {
-  const int numSpans = state.range(0);
+  const int num_spans = state.range(0);
   for (auto _ : state)
   {
     std::vector<opentelemetry::nostd::shared_ptr<opentelemetry::trace::Span>> running_spans;
-    std::thread run(StartSpans, std::ref(running_spans), std::ref(tracer), numSpans / 3, false);
-    std::thread err(StartEndSpansError, std::ref(tracer), numSpans / 3, false);
-    std::thread lat(StartEndSpansLatency, std::ref(tracer), numSpans / 3, false);
-    GetManyAggregations(std::ref(aggregator), numSpans / 3);
+    std::thread run(StartSpans, std::ref(running_spans), std::ref(tracer), num_spans / 3, false);
+    std::thread err(StartEndSpansError, std::ref(tracer), num_spans / 3, false);
+    std::thread lat(StartEndSpansLatency, std::ref(tracer), num_spans / 3, false);
+    GetManyAggregations(std::ref(aggregator), num_spans / 3);
     run.join();
     err.join();
     lat.join();
@@ -220,14 +220,14 @@ BENCHMARK_DEFINE_F(TracezAggregator, BM_BucketsSameGet)(benchmark::State &state)
 
 BENCHMARK_DEFINE_F(TracezAggregator, BM_BucketsUniqueGet)(benchmark::State &state)
 {
-  const int numSpans = state.range(0);
+  const int num_spans = state.range(0);
   for (auto _ : state)
   {
     std::vector<opentelemetry::nostd::shared_ptr<opentelemetry::trace::Span>> running_spans;
-    std::thread run(StartSpans, std::ref(running_spans), std::ref(tracer), numSpans / 3, true);
-    std::thread err(StartEndSpansError, std::ref(tracer), numSpans / 3, true);
-    std::thread lat(StartEndSpansLatency, std::ref(tracer), numSpans / 3, true);
-    GetManyAggregations(std::ref(aggregator), numSpans / 3);
+    std::thread run(StartSpans, std::ref(running_spans), std::ref(tracer), num_spans / 3, true);
+    std::thread err(StartEndSpansError, std::ref(tracer), num_spans / 3, true);
+    std::thread lat(StartEndSpansLatency, std::ref(tracer), num_spans / 3, true);
+    GetManyAggregations(std::ref(aggregator), num_spans / 3);
     run.join();
     err.join();
     lat.join();
diff --git a/ext/zpages/test/tracez_processor_benchmark.cc b/ext/zpages/test/tracez_processor_benchmark.cc
index 3781859e0a..ee793eeb78 100644
--- a/ext/zpages/test/tracez_processor_benchmark.cc
+++ b/ext/zpages/test/tracez_processor_benchmark.cc
@@ -75,10 +75,10 @@ class TracezProcessor : public benchmark::Fixture
  */
 BENCHMARK_DEFINE_F(TracezProcessor, BM_Run)(benchmark::State &state)
 {
-  const int numSpans = state.range(0);
+  const int num_spans = state.range(0);
   for (auto _ : state)
   {
-    StartManySpans(spans, std::ref(tracer), numSpans);
+    StartManySpans(spans, std::ref(tracer), num_spans);
   }
 }
 
@@ -88,10 +88,10 @@ BENCHMARK_DEFINE_F(TracezProcessor, BM_Run)(benchmark::State &state)
  */
 BENCHMARK_DEFINE_F(TracezProcessor, BM_Snap)(benchmark::State &state)
 {
-  const int numSpans = state.range(0);
+  const int num_spans = state.range(0);
   for (auto _ : state)
   {
-    GetManySnapshots(processor, numSpans);
+    GetManySnapshots(processor, num_spans);
   }
 }
 
@@ -101,13 +101,13 @@ BENCHMARK_DEFINE_F(TracezProcessor, BM_Snap)(benchmark::State &state)
  */
 BENCHMARK_DEFINE_F(TracezProcessor, BM_RunComplete)(benchmark::State &state)
 {
-  const int numSpans = state.range(0);
+  const int num_spans = state.range(0);
   for (auto _ : state)
   {
     std::vector<opentelemetry::nostd::shared_ptr<opentelemetry::trace::Span>> spans2;
-    StartManySpans(spans2, std::ref(tracer), numSpans);
+    StartManySpans(spans2, std::ref(tracer), num_spans);
 
-    std::thread start(StartManySpans, std::ref(spans), std::ref(tracer), numSpans);
+    std::thread start(StartManySpans, std::ref(spans), std::ref(tracer), num_spans);
     EndAllSpans(spans2);
 
     start.join();
@@ -122,11 +122,11 @@ BENCHMARK_DEFINE_F(TracezProcessor, BM_RunComplete)(benchmark::State &state)
  */
 BENCHMARK_DEFINE_F(TracezProcessor, BM_RunSnap)(benchmark::State &state)
 {
-  const int numSpans = state.range(0);
+  const int num_spans = state.range(0);
   for (auto _ : state)
   {
-    std::thread snapshots(GetManySnapshots, std::ref(processor), numSpans);
-    StartManySpans(spans, std::ref(tracer), numSpans);
+    std::thread snapshots(GetManySnapshots, std::ref(processor), num_spans);
+    StartManySpans(spans, std::ref(tracer), num_spans);
 
     snapshots.join();
     EndAllSpans(spans);
@@ -140,12 +140,12 @@ BENCHMARK_DEFINE_F(TracezProcessor, BM_RunSnap)(benchmark::State &state)
  */
 BENCHMARK_DEFINE_F(TracezProcessor, BM_SnapComplete)(benchmark::State &state)
 {
-  const int numSpans = state.range(0);
+  const int num_spans = state.range(0);
   for (auto _ : state)
   {
-    StartManySpans(spans, std::ref(tracer), numSpans);
+    StartManySpans(spans, std::ref(tracer), num_spans);
 
-    std::thread snapshots(GetManySnapshots, std::ref(processor), numSpans);
+    std::thread snapshots(GetManySnapshots, std::ref(processor), num_spans);
     EndAllSpans(spans);
 
     snapshots.join();
@@ -160,15 +160,15 @@ BENCHMARK_DEFINE_F(TracezProcessor, BM_SnapComplete)(benchmark::State &state)
  */
 BENCHMARK_DEFINE_F(TracezProcessor, BM_RunSnapComplete)(benchmark::State &state)
 {
-  const int numSpans = state.range(0);
+  const int num_spans = state.range(0);
   for (auto _ : state)
   {
     std::vector<opentelemetry::nostd::shared_ptr<opentelemetry::trace::Span>> spans2;
 
-    StartManySpans(spans, std::ref(tracer), numSpans);
+    StartManySpans(spans, std::ref(tracer), num_spans);
 
-    std::thread start(StartManySpans, std::ref(spans2), std::ref(tracer), numSpans);
-    std::thread snapshots(GetManySnapshots, std::ref(processor), numSpans);
+    std::thread start(StartManySpans, std::ref(spans2), std::ref(tracer), num_spans);
+    std::thread snapshots(GetManySnapshots, std::ref(processor), num_spans);
     EndAllSpans(spans);
 
     start.join();

From 0c5b994b3c0b58a04df47565b052d19663bb2601 Mon Sep 17 00:00:00 2001
From: Janet Vu <vjanet@umich.edu>
Date: Mon, 24 Aug 2020 16:46:08 +0000
Subject: [PATCH 23/40] Add more TODOs

---
 ext/zpages/README.md | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/ext/zpages/README.md b/ext/zpages/README.md
index 40df3d94f1..8d44aa15f5 100644
--- a/ext/zpages/README.md
+++ b/ext/zpages/README.md
@@ -1,6 +1,6 @@
 # zPages
 ## Overview
-zPages are a quick and light way to view tracing and metrics information on standard OpenTelemetry C++ instrumented applications. It requires no external dependencies. See more information in the OTel zPages experimental [spec](https://github.com/open-telemetry/opentelemetry-specification/blob/5b86d4b6c42e6d1e47d9155ac1e2e27f0f0b7769/experimental/trace/zpages.md).
+zPages are a quick and light way to view tracing and metrics information on standard OpenTelemetry C++ instrumented applications. It requires no external dependencies or backend setup. See more information in the OTel zPages experimental [spec](https://github.com/open-telemetry/opentelemetry-specification/blob/5b86d4b6c42e6d1e47d9155ac1e2e27f0f0b7769/experimental/trace/zpages.md). OTel C++ currently omly offers Tracez; future zPages to potentially add include TraceConfigz, RPCz, and Statsz.
 
 # Usage
 > TODO: Add CMake instructions

From 1e5c7a3d6c4f51df0564e4b2e1e60a0fc1beec61 Mon Sep 17 00:00:00 2001
From: Janet Vu <vjanet@umich.edu>
Date: Mon, 24 Aug 2020 16:48:17 +0000
Subject: [PATCH 24/40] Fix typo

---
 ext/zpages/README.md | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/ext/zpages/README.md b/ext/zpages/README.md
index 8d44aa15f5..36decdcbd6 100644
--- a/ext/zpages/README.md
+++ b/ext/zpages/README.md
@@ -1,6 +1,6 @@
 # zPages
 ## Overview
-zPages are a quick and light way to view tracing and metrics information on standard OpenTelemetry C++ instrumented applications. It requires no external dependencies or backend setup. See more information in the OTel zPages experimental [spec](https://github.com/open-telemetry/opentelemetry-specification/blob/5b86d4b6c42e6d1e47d9155ac1e2e27f0f0b7769/experimental/trace/zpages.md). OTel C++ currently omly offers Tracez; future zPages to potentially add include TraceConfigz, RPCz, and Statsz.
+zPages are a quick and light way to view tracing and metrics information on standard OpenTelemetry C++ instrumented applications. It requires no external dependencies or backend setup. See more information in the OTel zPages experimental [spec](https://github.com/open-telemetry/opentelemetry-specification/blob/5b86d4b6c42e6d1e47d9155ac1e2e27f0f0b7769/experimental/trace/zpages.md). OTel C++ currently only offers Tracez; future zPages to potentially add include TraceConfigz, RPCz, and Statsz.
 
 # Usage
 > TODO: Add CMake instructions
@@ -15,7 +15,7 @@ zPages are a quick and light way to view tracing and metrics information on stan
    bazel-bin/examples/zpages/zpages_example
    ```
    If you look at the zPages example's source [code](https://github.com/open-telemetry/opentelemetry-cpp/blob/master/examples/zpages/zpages_example.cc), it demonstrates adding zPages, manual application instrumentation (which sends data to zPages for viewing), and simulated use cases for zPages.
-3. View zPages ib http://localhost:3000/tracez
+3. View zPages at http://localhost:3000/tracez
 
 
 ## More Information

From 5c6c5f8ec557f9b6d965b240ff1cba0dd76e843a Mon Sep 17 00:00:00 2001
From: Janet Vu <vjanet@umich.edu>
Date: Mon, 24 Aug 2020 22:05:20 +0000
Subject: [PATCH 25/40] Update tracez todo in readme

---
 ext/zpages/README.md | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/ext/zpages/README.md b/ext/zpages/README.md
index 36decdcbd6..ef99fed913 100644
--- a/ext/zpages/README.md
+++ b/ext/zpages/README.md
@@ -1,6 +1,6 @@
 # zPages
 ## Overview
-zPages are a quick and light way to view tracing and metrics information on standard OpenTelemetry C++ instrumented applications. It requires no external dependencies or backend setup. See more information in the OTel zPages experimental [spec](https://github.com/open-telemetry/opentelemetry-specification/blob/5b86d4b6c42e6d1e47d9155ac1e2e27f0f0b7769/experimental/trace/zpages.md). OTel C++ currently only offers Tracez; future zPages to potentially add include TraceConfigz, RPCz, and Statsz.
+zPages are a quick and light way to view tracing and metrics information on standard OpenTelemetry C++ instrumented applications. It requires no external dependencies or backend setup. See more information in the OTel zPages experimental [spec](https://github.com/open-telemetry/opentelemetry-specification/blob/5b86d4b6c42e6d1e47d9155ac1e2e27f0f0b7769/experimental/trace/zpages.md). OTel C++ currently only offers Tracez; future zPages to potentially add include TraceConfigz, RPCz, and Statsz. Events and links need to be added to Tracez.
 
 # Usage
 > TODO: Add CMake instructions

From 92262de993416d93073eb50c606a5164195d3639 Mon Sep 17 00:00:00 2001
From: Janet Vu <vjanet@umich.edu>
Date: Tue, 25 Aug 2020 14:53:17 +0000
Subject: [PATCH 26/40] Be more descriptive, follow cpp style better

---
 .../test/tracez_data_aggregator_benchmark.cc  | 67 +++++++++++--------
 ext/zpages/test/tracez_processor_benchmark.cc | 33 ++++-----
 2 files changed, 55 insertions(+), 45 deletions(-)

diff --git a/ext/zpages/test/tracez_data_aggregator_benchmark.cc b/ext/zpages/test/tracez_data_aggregator_benchmark.cc
index e69f915b49..6e0fc24f8c 100644
--- a/ext/zpages/test/tracez_data_aggregator_benchmark.cc
+++ b/ext/zpages/test/tracez_data_aggregator_benchmark.cc
@@ -1,9 +1,9 @@
-#include "opentelemetry/context/threadlocal_context.h"
 #include "opentelemetry/ext/zpages/tracez_data_aggregator.h"
 
 #include <benchmark/benchmark.h>
 #include <chrono>
 
+#include "opentelemetry/context/threadlocal_context.h"
 #include "opentelemetry/sdk/trace/tracer.h"
 
 using namespace opentelemetry::sdk::trace;
@@ -13,25 +13,31 @@ using opentelemetry::core::SteadyTimestamp;
 /////////////////////////////// BENCHMARK HELPER FUNCTIONS //////////////////////////////
 
 /*
- * Helper function that creates and ends i spans instantly.
+ * Helper function that creates and ends i spans instantly. If is_unique is
+ * true, then all spans will have different names.
  */
 void StartEndSpans(
     std::shared_ptr<opentelemetry::trace::Tracer> &tracer,
     int i,
-    bool isUnique = false)
+    bool is_unique = false)
 {
+  opentelemetry::trace::StartSpanOptions start_end;
+  start_end.start_steady_time = SteadyTimestamp(nanoseconds(0));
   for (; i > 0; i--)
-    tracer->StartSpan(isUnique ? std::to_string(i) : "")->End();
+    tracer->StartSpan(isUnique ? std::to_string(i) : "", start_end)
+        ->End(start_end);
 }
 
 /*
  * Helper function that creates and ends spans instantly, while simulating
- * different latencies.
+ * different latencies. If is_unique is true, then all spans will have
+ * different names.
+
  */
 void StartEndSpansLatency(
     std::shared_ptr<opentelemetry::trace::Tracer> &tracer,
     int i,
-    bool isUnique = false)
+    bool is_unique = false)
 {
   opentelemetry::trace::StartSpanOptions start;
   start.start_steady_time = SteadyTimestamp(nanoseconds(0));
@@ -42,34 +48,36 @@ void StartEndSpansLatency(
     opentelemetry::trace::EndSpanOptions end;
     end.end_steady_time = SteadyTimestamp(latency_band);
 
-    tracer->StartSpan(isUnique ? std::to_string(i) : "", start)->End(end);
+    tracer->StartSpan(is_unique ? std::to_string(i) : "", start)->End(end);
   }
 }
+
 /*
  * Helper function that creates and ends spans instantly, while simulating
- * error codes.
+ * error codes. If is_unique is true, then all spans will have different names.
  */
 void StartEndSpansError(
     std::shared_ptr<opentelemetry::trace::Tracer> &tracer,
     int i,
-    bool isUnique = false)
+    bool is_unique = false)
 {
   for (; i > 0; i--)
-    tracer->StartSpan(isUnique ? std::to_string(i) : "")
+    tracer->StartSpan(is_unique ? std::to_string(i) : "")
         ->SetStatus(opentelemetry::trace::CanonicalCode::CANCELLED, "");
 }
 
 /*
- * Helper function that creates i spans that always run.
+ * Helper function that creates i spans that always run. If is_unique is true,
+ * then all spans will have different names.
  */
 void StartSpans(
     std::vector<opentelemetry::nostd::shared_ptr<opentelemetry::trace::Span>> &spans,
     std::shared_ptr<opentelemetry::trace::Tracer> &tracer,
     int i,
-    bool isUnique = false)
+    bool is_unique = false)
 {
   for (; i > 0; i--)
-    spans.push_back(tracer->StartSpan(isUnique ? std::to_string(i) : ""));
+    spans.push_back(tracer->StartSpan(is_unique ? std::to_string(i) : ""));
 }
 
 /*
@@ -112,7 +120,7 @@ class TracezAggregator : public benchmark::Fixture
  * of latencies is required, as all spans should be sorted in the same bucket
  * under the same span name.
  */
-BENCHMARK_DEFINE_F(TracezAggregator, BM_InstantSame)(benchmark::State &state)
+BENCHMARK_DEFINE_F(TracezAggregator, BM_SingleBucketSingleName)(benchmark::State &state)
 {
   const int num_spans = state.range(0);
   for (auto _ : state)
@@ -127,7 +135,7 @@ BENCHMARK_DEFINE_F(TracezAggregator, BM_InstantSame)(benchmark::State &state)
  * of latencies is required. Spans are sorted in different groups but always
  * in the same bucket.
  */
-BENCHMARK_DEFINE_F(TracezAggregator, BM_InstantUnique)(benchmark::State &state)
+BENCHMARK_DEFINE_F(TracezAggregator, BM_SingleBucketManyNames)(benchmark::State &state)
 {
   const int num_spans = state.range(0);
   for (auto _ : state)
@@ -143,7 +151,7 @@ BENCHMARK_DEFINE_F(TracezAggregator, BM_InstantUnique)(benchmark::State &state)
  * respective buckets. Spans are in the same group but sorted to different
  * buckets.
  */
-BENCHMARK_DEFINE_F(TracezAggregator, BM_BucketsSame)(benchmark::State &state)
+BENCHMARK_DEFINE_F(TracezAggregator, BM_ManyBucketsSingleName)(benchmark::State &state)
 {
   const int num_spans = state.range(0);
   for (auto _ : state)
@@ -164,7 +172,7 @@ BENCHMARK_DEFINE_F(TracezAggregator, BM_BucketsSame)(benchmark::State &state)
  * respective buckets. Spans are in the different groups and buckets, similar to
  * likely real use cases.
  */
-BENCHMARK_DEFINE_F(TracezAggregator, BM_BucketsUnique)(benchmark::State &state)
+BENCHMARK_DEFINE_F(TracezAggregator, BM_ManyBucketsManyNames)(benchmark::State &state)
 {
   const int num_spans = state.range(0);
   for (auto _ : state)
@@ -180,7 +188,7 @@ BENCHMARK_DEFINE_F(TracezAggregator, BM_BucketsUnique)(benchmark::State &state)
 
 //////////////////// SAME BENCHMARKS, BUT USER VISITS WEBPAGE ////////////////////////
 
-BENCHMARK_DEFINE_F(TracezAggregator, BM_InstantSameGet)(benchmark::State &state)
+BENCHMARK_DEFINE_F(TracezAggregator, BM_SingleBucketSingleNameGet)(benchmark::State &state)
 {
   const int num_spans = state.range(0);
   for (auto _ : state)
@@ -191,7 +199,7 @@ BENCHMARK_DEFINE_F(TracezAggregator, BM_InstantSameGet)(benchmark::State &state)
   }
 }
 
-BENCHMARK_DEFINE_F(TracezAggregator, BM_InstantUniqueGet)(benchmark::State &state)
+BENCHMARK_DEFINE_F(TracezAggregator, BM_SingleBucketManyNamesGet)(benchmark::State &state)
 {
   const int num_spans = state.range(0);
   for (auto _ : state)
@@ -202,7 +210,7 @@ BENCHMARK_DEFINE_F(TracezAggregator, BM_InstantUniqueGet)(benchmark::State &stat
   }
 }
 
-BENCHMARK_DEFINE_F(TracezAggregator, BM_BucketsSameGet)(benchmark::State &state)
+BENCHMARK_DEFINE_F(TracezAggregator, BM_ManyBucketsSingleNameGet)(benchmark::State &state)
 {
   const int num_spans = state.range(0);
   for (auto _ : state)
@@ -218,7 +226,7 @@ BENCHMARK_DEFINE_F(TracezAggregator, BM_BucketsSameGet)(benchmark::State &state)
   }
 }
 
-BENCHMARK_DEFINE_F(TracezAggregator, BM_BucketsUniqueGet)(benchmark::State &state)
+BENCHMARK_DEFINE_F(TracezAggregator, BM_ManyBucketsManyNamesGet)(benchmark::State &state)
 {
   const int num_spans = state.range(0);
   for (auto _ : state)
@@ -236,14 +244,15 @@ BENCHMARK_DEFINE_F(TracezAggregator, BM_BucketsUniqueGet)(benchmark::State &stat
 
 //////////////////////////// RUN BENCHMARKS ///////////////////////////////
 
-BENCHMARK_REGISTER_F(TracezAggregator, BM_InstantSame)->Arg(10)->Arg(1000);
-BENCHMARK_REGISTER_F(TracezAggregator, BM_InstantSameGet)->Arg(10)->Arg(1000);
-BENCHMARK_REGISTER_F(TracezAggregator, BM_InstantUnique)->Arg(10)->Arg(1000);
-BENCHMARK_REGISTER_F(TracezAggregator, BM_InstantUniqueGet)->Arg(10)->Arg(1000);
-BENCHMARK_REGISTER_F(TracezAggregator, BM_BucketsSame)->Arg(10)->Arg(1000);
-BENCHMARK_REGISTER_F(TracezAggregator, BM_BucketsSameGet)->Arg(10)->Arg(1000);
-BENCHMARK_REGISTER_F(TracezAggregator, BM_BucketsUnique)->Arg(10)->Arg(1000);
-BENCHMARK_REGISTER_F(TracezAggregator, BM_BucketsUniqueGet)->Arg(10)->Arg(1000);
+// Arg is the number of spans created for each iteration
+BENCHMARK_REGISTER_F(TracezAggregator, BM_SingleBucketSingleName)->Arg(10)->Arg(1000);
+BENCHMARK_REGISTER_F(TracezAggregator, BM_SingleBucketSingleNameGet)->Arg(10)->Arg(1000);
+BENCHMARK_REGISTER_F(TracezAggregator, BM_SingleBucketManyNames)->Arg(10)->Arg(1000);
+BENCHMARK_REGISTER_F(TracezAggregator, BM_SingleBucketManyNamesGet)->Arg(10)->Arg(1000);
+BENCHMARK_REGISTER_F(TracezAggregator, BM_ManyBucketsSingleName)->Arg(10)->Arg(1000);
+BENCHMARK_REGISTER_F(TracezAggregator, BM_ManyBucketsSingleNameGet)->Arg(10)->Arg(1000);
+BENCHMARK_REGISTER_F(TracezAggregator, BM_ManyBucketsManyNames)->Arg(10)->Arg(1000);
+BENCHMARK_REGISTER_F(TracezAggregator, BM_ManyBucketsManyNamesGet)->Arg(10)->Arg(1000);
 
 BENCHMARK_MAIN();
 
diff --git a/ext/zpages/test/tracez_processor_benchmark.cc b/ext/zpages/test/tracez_processor_benchmark.cc
index ee793eeb78..5ceecd7f81 100644
--- a/ext/zpages/test/tracez_processor_benchmark.cc
+++ b/ext/zpages/test/tracez_processor_benchmark.cc
@@ -1,9 +1,9 @@
-#include "opentelemetry/context/threadlocal_context.h"
 #include "opentelemetry/ext/zpages/tracez_processor.h"
 
 #include <benchmark/benchmark.h>
 #include <thread>
 
+#include "opentelemetry/context/threadlocal_context.h"
 #include "opentelemetry/sdk/trace/tracer.h"
 
 using namespace opentelemetry::sdk::trace;
@@ -16,7 +16,7 @@ using namespace opentelemetry::ext::zpages;
  */
 void StartManySpans(
     std::vector<opentelemetry::nostd::shared_ptr<opentelemetry::trace::Span>> &spans,
-    std::shared_ptr<opentelemetry::trace::Tracer> &tracer,
+    std::unique_ptr<opentelemetry::trace::Tracer> &tracer,
     int i)
 {
   for (; i > 0; i--)
@@ -58,11 +58,11 @@ class TracezProcessor : public benchmark::Fixture
   void SetUp(const ::benchmark::State& state)
   {
     processor  = std::shared_ptr<TracezSpanProcessor>(new TracezSpanProcessor());
-    tracer     = std::shared_ptr<opentelemetry::trace::Tracer>(new Tracer(processor));
+    tracer     = std::unique_ptr<opentelemetry::trace::Tracer>(new Tracer(processor));
   }
 
   std::shared_ptr<TracezSpanProcessor> processor;
-  std::shared_ptr<opentelemetry::trace::Tracer> tracer;
+  std::unique_ptr<opentelemetry::trace::Tracer> tracer;
 
   std::vector<opentelemetry::nostd::shared_ptr<opentelemetry::trace::Span>> spans;
 };
@@ -73,7 +73,7 @@ class TracezProcessor : public benchmark::Fixture
  * Make many empty spans. This checks the scenario where the processor holds
  * many running spans but never gets queried.
  */
-BENCHMARK_DEFINE_F(TracezProcessor, BM_Run)(benchmark::State &state)
+BENCHMARK_DEFINE_F(TracezProcessor, BM_MakeRunning)(benchmark::State &state)
 {
   const int num_spans = state.range(0);
   for (auto _ : state)
@@ -86,7 +86,7 @@ BENCHMARK_DEFINE_F(TracezProcessor, BM_Run)(benchmark::State &state)
  * Make many snapshots. This checks the scenario where the processor holds
  * no spans but gets queried many times.
  */
-BENCHMARK_DEFINE_F(TracezProcessor, BM_Snap)(benchmark::State &state)
+BENCHMARK_DEFINE_F(TracezProcessor, BM_GetSpans)(benchmark::State &state)
 {
   const int num_spans = state.range(0);
   for (auto _ : state)
@@ -99,7 +99,7 @@ BENCHMARK_DEFINE_F(TracezProcessor, BM_Snap)(benchmark::State &state)
  * Make and end many empty spans. This checks the scenario where the processor holds
  * many running and completed spans but never gets queried.
  */
-BENCHMARK_DEFINE_F(TracezProcessor, BM_RunComplete)(benchmark::State &state)
+BENCHMARK_DEFINE_F(TracezProcessor, BM_MakeRunningMakeComplete)(benchmark::State &state)
 {
   const int num_spans = state.range(0);
   for (auto _ : state)
@@ -120,7 +120,7 @@ BENCHMARK_DEFINE_F(TracezProcessor, BM_RunComplete)(benchmark::State &state)
  * Make many empty spans while spapshots grabbed. This checks the scenario where the
  * processor holds many running spans and gets queried.
  */
-BENCHMARK_DEFINE_F(TracezProcessor, BM_RunSnap)(benchmark::State &state)
+BENCHMARK_DEFINE_F(TracezProcessor, BM_MakeRunningGetSpans)(benchmark::State &state)
 {
   const int num_spans = state.range(0);
   for (auto _ : state)
@@ -138,7 +138,7 @@ BENCHMARK_DEFINE_F(TracezProcessor, BM_RunSnap)(benchmark::State &state)
  * where the processor doesn't make new spans, but existing spans complete while they're
  * queried.
  */
-BENCHMARK_DEFINE_F(TracezProcessor, BM_SnapComplete)(benchmark::State &state)
+BENCHMARK_DEFINE_F(TracezProcessor, BM_GetSpansMakeComplete)(benchmark::State &state)
 {
   const int num_spans = state.range(0);
   for (auto _ : state)
@@ -158,7 +158,7 @@ BENCHMARK_DEFINE_F(TracezProcessor, BM_SnapComplete)(benchmark::State &state)
  * checks the scenario where the processor makes new spans, other spans complete,
  * and all spans are queried. This is the case most similar to real situations.
  */
-BENCHMARK_DEFINE_F(TracezProcessor, BM_RunSnapComplete)(benchmark::State &state)
+BENCHMARK_DEFINE_F(TracezProcessor, BM_MakeRunningGetSpansMakeComplete)(benchmark::State &state)
 {
   const int num_spans = state.range(0);
   for (auto _ : state)
@@ -180,12 +180,13 @@ BENCHMARK_DEFINE_F(TracezProcessor, BM_RunSnapComplete)(benchmark::State &state)
 
 /////////////////////// RUN BENCHMARKS ///////////////////////////
 
-BENCHMARK_REGISTER_F(TracezProcessor, BM_Run)->Arg(10)->Arg(1000);
-BENCHMARK_REGISTER_F(TracezProcessor, BM_Snap)->Arg(10)->Arg(1000);
-BENCHMARK_REGISTER_F(TracezProcessor, BM_RunComplete)->Arg(10)->Arg(1000);
-BENCHMARK_REGISTER_F(TracezProcessor, BM_RunSnap)->Arg(10)->Arg(1000);
-BENCHMARK_REGISTER_F(TracezProcessor, BM_SnapComplete)->Arg(10)->Arg(1000);
-BENCHMARK_REGISTER_F(TracezProcessor, BM_RunSnapComplete)->Arg(10)->Arg(1000);
+// Arg is the number of spans created for each iteration
+BENCHMARK_REGISTER_F(TracezProcessor, BM_MakeRunning)->Arg(10)->Arg(1000);
+BENCHMARK_REGISTER_F(TracezProcessor, BM_GetSpans)->Arg(10)->Arg(1000);
+BENCHMARK_REGISTER_F(TracezProcessor, BM_MakeRunningMakeComplete)->Arg(10)->Arg(1000);
+BENCHMARK_REGISTER_F(TracezProcessor, BM_MakeRunningGetSpans)->Arg(10)->Arg(1000);
+BENCHMARK_REGISTER_F(TracezProcessor, BM_GetSpansMakeComplete)->Arg(10)->Arg(1000);
+BENCHMARK_REGISTER_F(TracezProcessor, BM_MakeRunningGetSpansMakeComplete)->Arg(10)->Arg(1000);
 
 BENCHMARK_MAIN();
 

From 6381880d3eee3eab1d1d69b4b123580208f908e9 Mon Sep 17 00:00:00 2001
From: Janet Vu <vjanet@umich.edu>
Date: Wed, 26 Aug 2020 14:31:11 +0000
Subject: [PATCH 27/40] Use friend classes, granulate aggr fixture

---
 .../ext/zpages/tracez_data_aggregator.h       |   3 +
 .../ext/zpages/tracez_processor.h             |   3 +
 .../test/tracez_data_aggregator_benchmark.cc  | 253 +++++++++++++-----
 ext/zpages/test/tracez_processor_benchmark.cc | 138 ++++++----
 4 files changed, 287 insertions(+), 110 deletions(-)

diff --git a/ext/zpages/include/opentelemetry/ext/zpages/tracez_data_aggregator.h b/ext/zpages/include/opentelemetry/ext/zpages/tracez_data_aggregator.h
index 6d2d0a054e..d8de9c467d 100644
--- a/ext/zpages/include/opentelemetry/ext/zpages/tracez_data_aggregator.h
+++ b/ext/zpages/include/opentelemetry/ext/zpages/tracez_data_aggregator.h
@@ -161,6 +161,9 @@ class TracezDataAggregator
   /** Condition variable that notifies the thread when object is about to be
   destroyed **/
   std::condition_variable cv_;
+
+  /** Friend class used for benchmarking **/
+  friend class TracezDataAggregatorPeer;
 };
 
 }  // namespace zpages
diff --git a/ext/zpages/include/opentelemetry/ext/zpages/tracez_processor.h b/ext/zpages/include/opentelemetry/ext/zpages/tracez_processor.h
index c96461db74..6f35b2bb46 100644
--- a/ext/zpages/include/opentelemetry/ext/zpages/tracez_processor.h
+++ b/ext/zpages/include/opentelemetry/ext/zpages/tracez_processor.h
@@ -91,6 +91,9 @@ class TracezSpanProcessor : public opentelemetry::sdk::trace::SpanProcessor
 private:
   mutable std::mutex mtx_;
   CollectedSpans spans_;
+
+  /** Friend class used for benchmarking **/
+  friend class TracezProcessorPeer;
 };
 }  // namespace zpages
 }  // namespace ext
diff --git a/ext/zpages/test/tracez_data_aggregator_benchmark.cc b/ext/zpages/test/tracez_data_aggregator_benchmark.cc
index 6e0fc24f8c..ebf3f450ca 100644
--- a/ext/zpages/test/tracez_data_aggregator_benchmark.cc
+++ b/ext/zpages/test/tracez_data_aggregator_benchmark.cc
@@ -2,14 +2,20 @@
 
 #include <benchmark/benchmark.h>
 #include <chrono>
+#include <iostream>
 
 #include "opentelemetry/context/threadlocal_context.h"
 #include "opentelemetry/sdk/trace/tracer.h"
 
 using namespace opentelemetry::sdk::trace;
-using namespace opentelemetry::ext::zpages;
 using opentelemetry::core::SteadyTimestamp;
 
+OPENTELEMETRY_BEGIN_NAMESPACE
+namespace ext
+{
+namespace zpages
+{
+
 /////////////////////////////// BENCHMARK HELPER FUNCTIONS //////////////////////////////
 
 /*
@@ -21,18 +27,18 @@ void StartEndSpans(
     int i,
     bool is_unique = false)
 {
-  opentelemetry::trace::StartSpanOptions start_end;
-  start_end.start_steady_time = SteadyTimestamp(nanoseconds(0));
+  opentelemetry::trace::StartSpanOptions start;
+  opentelemetry::trace::EndSpanOptions end;
+  start.start_steady_time = SteadyTimestamp(nanoseconds(0));
+  end.end_steady_time = SteadyTimestamp(nanoseconds(1));
   for (; i > 0; i--)
-    tracer->StartSpan(isUnique ? std::to_string(i) : "", start_end)
-        ->End(start_end);
+    tracer->StartSpan(is_unique ? std::to_string(i) : "", start)->End(end);
 }
 
 /*
  * Helper function that creates and ends spans instantly, while simulating
  * different latencies. If is_unique is true, then all spans will have
  * different names.
-
  */
 void StartEndSpansLatency(
     std::shared_ptr<opentelemetry::trace::Tracer> &tracer,
@@ -67,8 +73,9 @@ void StartEndSpansError(
 }
 
 /*
- * Helper function that creates i spans that always run. If is_unique is true,
- * then all spans will have different names.
+ * Helper function that creates i spans that always run. To remain in scope and running,
+ * spans are referenced in the passed in vector. If is_unique is true, then all spans
+ * will have different names.
  */
 void StartSpans(
     std::vector<opentelemetry::nostd::shared_ptr<opentelemetry::trace::Span>> &spans,
@@ -81,35 +88,150 @@ void StartSpans(
 }
 
 /*
- * Helper function that queries for Tracez data. This simulates a user visiting
- * or refreshing the Tracez webpage many times, except this function does and
- * holds nothing in memory. While this runs, aggregation work and performance is
- * expected is be affected negatively.
+ * Helper function that creates about i spans, evenly split between running, latency (which
+ * is further split by bucket), and error. If is_unique is true, then all spans will have
+ * different names.
  */
-
-void GetManyAggregations(std::unique_ptr<TracezDataAggregator> &aggregator, int i)
+std::vector<opentelemetry::nostd::shared_ptr<opentelemetry::trace::Span>> MakeManySpans(
+    std::shared_ptr<opentelemetry::trace::Tracer> &tracer,
+    int num_spans,
+    bool is_unique = false)
 {
-  for (; i > 0; i--)
-    aggregator->GetAggregatedTracezData();
+  // Running spans must be stored in a vector in order to stay running
+  std::vector<opentelemetry::nostd::shared_ptr<opentelemetry::trace::Span>> running_spans;
+  // Use threads to speed up the work
+  std::thread run(StartSpans, std::ref(running_spans), std::ref(tracer), num_spans / 3, is_unique);
+  std::thread err(StartEndSpansError, std::ref(tracer), num_spans / 3, is_unique);
+  StartEndSpansLatency(tracer, num_spans / 3, is_unique);
+  run.join();
+  err.join();
+  return running_spans;
 }
 
+
+/////////////////////////// AGGREGATOR PEER //////////////////////////////
+
+/*
+ * Friend class allows us to access aggregator private variables for
+ * benchmarking and isolation of different functions
+ */
+class TracezDataAggregatorPeer
+{
+public:
+  TracezDataAggregatorPeer(std::shared_ptr<TracezSpanProcessor> processor)
+  {
+    // Set up the aggregator
+    aggregator = std::unique_ptr<TracezDataAggregator>(
+        new TracezDataAggregator(processor));
+
+    // Disable the aggregetor's periodic background thread aggregation work, which
+    // it normally does during production. Disabling it allows us to isolate
+    // aggregation work for benchmarking
+    if (aggregator->execute_.load(std::memory_order_acquire))
+    {
+      aggregator->execute_.store(false, std::memory_order_release);
+      aggregator->cv_.notify_one();
+      aggregator->aggregate_spans_thread_.join();
+    }
+  }
+
+  /*
+   * Join and end periodic query loop in background if it's running.
+   */
+  ~TracezDataAggregatorPeer()
+  {
+    if (run.load(std::memory_order_acquire))
+    {
+      run.store(false, std::memory_order_release);
+      query_thread.join();
+    }
+  }
+
+  /*
+   * Calling the aggregation work function, locking as needed.
+   */
+  void Aggregate()
+  {
+    std::lock_guard<std::mutex> lock(mtx);
+    aggregator->AggregateSpans();
+  }
+
+  /*
+   * Starts a background thread to query for aggregated data every i
+   * nanoseconds. This should affect performance for the aggregator,
+   * since aggregation work cannot be done while aggegation data is being
+   * requested. This simulates a user visiting or refreshing the Tracez
+   * webpage many times, except this function does and holds nothing in
+   * memory.
+   */
+  void StartPeriodicQuery(int i = 1)
+  {
+    run.store(true, std::memory_order_release);
+    nanoseconds query_interval = nanoseconds(i);
+    query_thread = std::thread([this, query_interval]() {
+      while (run.load(std::memory_order_acquire))
+      {
+        std::unique_lock<std::mutex> lock(mtx);
+        aggregator->GetAggregatedTracezData();
+        // Continue if mutex is free and query interval passed
+        cont.wait_for(lock, query_interval);
+      }
+    });
+  }
+
+private:
+  std::unique_ptr<TracezDataAggregator> aggregator;
+  // Keep queries running while peer is in memory
+  std::thread query_thread;
+  // Ensure queries and aggregations don't happen simultaneously
+  std::mutex mtx;
+  std::atomic<bool> run;
+  std::condition_variable cont;
+  
+};
+
 ////////////////////////  FIXTURE FOR SHARED SETUP CODE ///////////////////
 
+/*
+ * This is a regular fixture that initializes the shared needed components
+ * to create spans and do the aggregation work for benchmarking.
+ */
 class TracezAggregator : public benchmark::Fixture
 {
 protected:
-  void SetUp(const ::benchmark::State& state)
+  void SetTracerAggregatorPeer()
   {
     std::shared_ptr<TracezSpanProcessor> processor(new TracezSpanProcessor());
-    tracer     = std::shared_ptr<opentelemetry::trace::Tracer>(new Tracer(processor));
-    aggregator = std::unique_ptr<TracezDataAggregator>(
-        new TracezDataAggregator(processor, milliseconds(10)));
+    tracer = std::shared_ptr<opentelemetry::trace::Tracer>(new Tracer(processor));
+    aggregator_peer = std::unique_ptr<TracezDataAggregatorPeer>(
+        new TracezDataAggregatorPeer(processor));
   }
 
-  std::unique_ptr<TracezDataAggregator> aggregator;
+  void SetUp(const ::benchmark::State& state)
+  {
+    SetTracerAggregatorPeer();
+  }
+
+  // Aggregator peer for accessing private aggregator functions
+  std::unique_ptr<TracezDataAggregatorPeer> aggregator_peer;
+  // Tracer for creating spans
   std::shared_ptr<opentelemetry::trace::Tracer> tracer;
 };
 
+/*
+ * This extend the previous fixture, adding in the periodic query work in
+ * the background to simulate users requesting the data on the frontend
+ */
+class TracezAggregatorFetch : public TracezAggregator
+{
+protected:
+  void SetUp(const ::benchmark::State& state) override
+  {
+    SetTracerAggregatorPeer();
+    aggregator_peer->StartPeriodicQuery();
+  }
+};
+
 //////////////////////////// BENCHMARK DEFINITIONS /////////////////////////////////
 
 //////////////////////// USER NEVER VISITS WEBPAGE //////////////////////////////
@@ -125,7 +247,11 @@ BENCHMARK_DEFINE_F(TracezAggregator, BM_SingleBucketSingleName)(benchmark::State
   const int num_spans = state.range(0);
   for (auto _ : state)
   {
+    // Do not time span creation, as we're only benchmarking aggregation work
+    state.PauseTiming();
     StartEndSpans(tracer, num_spans);
+    state.ResumeTiming();
+    aggregator_peer->Aggregate();
   }
 }
 
@@ -140,7 +266,10 @@ BENCHMARK_DEFINE_F(TracezAggregator, BM_SingleBucketManyNames)(benchmark::State
   const int num_spans = state.range(0);
   for (auto _ : state)
   {
-    StartEndSpans(tracer, num_spans, true);
+    state.PauseTiming();
+    StartEndSpans(tracer, num_spans, false);
+    state.ResumeTiming();
+    aggregator_peer->Aggregate();
   }
 }
 
@@ -156,12 +285,10 @@ BENCHMARK_DEFINE_F(TracezAggregator, BM_ManyBucketsSingleName)(benchmark::State
   const int num_spans = state.range(0);
   for (auto _ : state)
   {
-    std::vector<opentelemetry::nostd::shared_ptr<opentelemetry::trace::Span>> running_spans;
-    std::thread run(StartSpans, std::ref(running_spans), std::ref(tracer), num_spans / 3, false);
-    std::thread err(StartEndSpansError, std::ref(tracer), num_spans / 3, false);
-    StartEndSpansLatency(tracer, num_spans / 3);
-    run.join();
-    err.join();
+    state.PauseTiming();
+    auto running_spans = MakeManySpans(tracer, num_spans, false);
+    state.ResumeTiming();
+    aggregator_peer->Aggregate();
   }
 }
 
@@ -177,68 +304,60 @@ BENCHMARK_DEFINE_F(TracezAggregator, BM_ManyBucketsManyNames)(benchmark::State &
   const int num_spans = state.range(0);
   for (auto _ : state)
   {
-    std::vector<opentelemetry::nostd::shared_ptr<opentelemetry::trace::Span>> running_spans;
-    std::thread run(StartSpans, std::ref(running_spans), std::ref(tracer), num_spans / 3, true);
-    std::thread err(StartEndSpansError, std::ref(tracer), num_spans / 3, true);
-    StartEndSpansLatency(tracer, num_spans / 3, true);
-    run.join();
-    err.join();
+    state.PauseTiming();
+    auto running_spans = MakeManySpans(tracer, num_spans, true);
+    state.ResumeTiming();
+    aggregator_peer->Aggregate();
   }
 }
 
-//////////////////// SAME BENCHMARKS, BUT USER VISITS WEBPAGE ////////////////////////
+//////////////////////// USER VISITS WEBPAGE //////////////////////////////
 
-BENCHMARK_DEFINE_F(TracezAggregator, BM_SingleBucketSingleNameGet)(benchmark::State &state)
+BENCHMARK_DEFINE_F(TracezAggregatorFetch, BM_SingleBucketSingleName)(benchmark::State &state)
 {
   const int num_spans = state.range(0);
   for (auto _ : state)
   {
-    std::thread spans(StartEndSpans, std::ref(tracer), num_spans, false);
-    GetManyAggregations(std::ref(aggregator), num_spans);
-    spans.join();
+    state.PauseTiming();
+    StartEndSpans(tracer, num_spans);
+    state.ResumeTiming();
+    aggregator_peer->Aggregate();
   }
 }
 
-BENCHMARK_DEFINE_F(TracezAggregator, BM_SingleBucketManyNamesGet)(benchmark::State &state)
+BENCHMARK_DEFINE_F(TracezAggregatorFetch, BM_SingleBucketManyNames)(benchmark::State &state)
 {
   const int num_spans = state.range(0);
   for (auto _ : state)
   {
-    std::thread spans(StartEndSpans, std::ref(tracer), num_spans, true);
-    GetManyAggregations(std::ref(aggregator), num_spans);
-    spans.join();
+    state.PauseTiming();
+    StartEndSpans(tracer, num_spans, false);
+    state.ResumeTiming();
+    aggregator_peer->Aggregate();
   }
 }
 
-BENCHMARK_DEFINE_F(TracezAggregator, BM_ManyBucketsSingleNameGet)(benchmark::State &state)
+BENCHMARK_DEFINE_F(TracezAggregatorFetch, BM_ManyBucketsSingleName)(benchmark::State &state)
 {
   const int num_spans = state.range(0);
   for (auto _ : state)
   {
-    std::vector<opentelemetry::nostd::shared_ptr<opentelemetry::trace::Span>> running_spans;
-    std::thread run(StartSpans, std::ref(running_spans), std::ref(tracer), num_spans / 3, false);
-    std::thread err(StartEndSpansError, std::ref(tracer), num_spans / 3, false);
-    std::thread lat(StartEndSpansLatency, std::ref(tracer), num_spans / 3, false);
-    GetManyAggregations(std::ref(aggregator), num_spans / 3);
-    run.join();
-    err.join();
-    lat.join();
+    state.PauseTiming();
+    auto running_spans = MakeManySpans(tracer, num_spans);
+    state.ResumeTiming();
+    aggregator_peer->Aggregate();
   }
 }
 
-BENCHMARK_DEFINE_F(TracezAggregator, BM_ManyBucketsManyNamesGet)(benchmark::State &state)
+BENCHMARK_DEFINE_F(TracezAggregatorFetch, BM_ManyBucketsManyNames)(benchmark::State &state)
 {
   const int num_spans = state.range(0);
   for (auto _ : state)
   {
-    std::vector<opentelemetry::nostd::shared_ptr<opentelemetry::trace::Span>> running_spans;
-    std::thread run(StartSpans, std::ref(running_spans), std::ref(tracer), num_spans / 3, true);
-    std::thread err(StartEndSpansError, std::ref(tracer), num_spans / 3, true);
-    std::thread lat(StartEndSpansLatency, std::ref(tracer), num_spans / 3, true);
-    GetManyAggregations(std::ref(aggregator), num_spans / 3);
-    run.join();
-    err.join();
-    lat.join();
+    state.PauseTiming();
+    auto running_spans = MakeManySpans(tracer, num_spans, true);
+    state.ResumeTiming();
+    aggregator_peer->Aggregate();
   }
 }
 
@@ -246,13 +365,17 @@ BENCHMARK_DEFINE_F(TracezAggregator, BM_ManyBucketsManyNamesGet)(benchmark::Stat
 
 // Arg is the number of spans created for each iteration
 BENCHMARK_REGISTER_F(TracezAggregator, BM_SingleBucketSingleName)->Arg(10)->Arg(1000);
-BENCHMARK_REGISTER_F(TracezAggregator, BM_SingleBucketSingleNameGet)->Arg(10)->Arg(1000);
+BENCHMARK_REGISTER_F(TracezAggregatorFetch, BM_SingleBucketSingleName)->Arg(10)->Arg(1000);
 BENCHMARK_REGISTER_F(TracezAggregator, BM_SingleBucketManyNames)->Arg(10)->Arg(1000);
-BENCHMARK_REGISTER_F(TracezAggregator, BM_SingleBucketManyNamesGet)->Arg(10)->Arg(1000);
+BENCHMARK_REGISTER_F(TracezAggregatorFetch, BM_SingleBucketManyNames)->Arg(10)->Arg(1000);
 BENCHMARK_REGISTER_F(TracezAggregator, BM_ManyBucketsSingleName)->Arg(10)->Arg(1000);
-BENCHMARK_REGISTER_F(TracezAggregator, BM_ManyBucketsSingleNameGet)->Arg(10)->Arg(1000);
+BENCHMARK_REGISTER_F(TracezAggregatorFetch, BM_ManyBucketsSingleName)->Arg(10)->Arg(1000);
 BENCHMARK_REGISTER_F(TracezAggregator, BM_ManyBucketsManyNames)->Arg(10)->Arg(1000);
-BENCHMARK_REGISTER_F(TracezAggregator, BM_ManyBucketsManyNamesGet)->Arg(10)->Arg(1000);
+BENCHMARK_REGISTER_F(TracezAggregatorFetch, BM_ManyBucketsManyNames)->Arg(10)->Arg(1000);
+
+} // namespace zpages
+} // namespace ext
+OPENTELEMETRY_END_NAMESPACE
 
 BENCHMARK_MAIN();
 
diff --git a/ext/zpages/test/tracez_processor_benchmark.cc b/ext/zpages/test/tracez_processor_benchmark.cc
index 5ceecd7f81..a9e57ce18c 100644
--- a/ext/zpages/test/tracez_processor_benchmark.cc
+++ b/ext/zpages/test/tracez_processor_benchmark.cc
@@ -4,37 +4,27 @@
 #include <thread>
 
 #include "opentelemetry/context/threadlocal_context.h"
-#include "opentelemetry/sdk/trace/tracer.h"
 
 using namespace opentelemetry::sdk::trace;
-using namespace opentelemetry::ext::zpages;
+
+OPENTELEMETRY_BEGIN_NAMESPACE
+namespace ext
+{
+namespace zpages
+{
 
 /////////////////////////////// BENCHMARK HELPER FUNCTIONS //////////////////////////////
 
 /*
- * Helper function that creates and add i spans into the passed in vector.
+ * Helper function that creates i threadsafe span data, adding them to the spans
+ * vector during setup to benchmark OnStart and OnEnd times for the processor.
  */
-void StartManySpans(
-    std::vector<opentelemetry::nostd::shared_ptr<opentelemetry::trace::Span>> &spans,
-    std::unique_ptr<opentelemetry::trace::Tracer> &tracer,
+void CreateRecordables(
+    std::vector<std::unique_ptr<ThreadsafeSpanData>> &spans,
     int i)
 {
   for (; i > 0; i--)
-    spans.push_back(tracer->StartSpan(""));
-}
-
-/*
- * Helper function that ends and removes all spans in the passed in span vector,
- * since they'll be in the processor memory.
- */
-void EndAllSpans(std::vector<opentelemetry::nostd::shared_ptr<opentelemetry::trace::Span>> &spans)
-{
-  while(!spans.empty())
-  {
-    auto span = spans.back();
-    span->End();
-    spans.pop_back();
-  }
+    spans.push_back(std::unique_ptr<ThreadsafeSpanData>(new ThreadsafeSpanData()));
 }
 
 /*
@@ -50,6 +40,44 @@ void GetManySnapshots(std::shared_ptr<TracezSpanProcessor> &processor, int i)
     processor->GetSpanSnapshot();
 }
 
+/////////////////////////// PROCESSOR PEER //////////////////////////////
+
+/*
+ * Friend class allows us to access processor private variables for
+ * benchmarking. It also reduces allows isolation of processor functions
+ * to minimize benchmarking noise, since we don't do the extra computation
+ * of setting variables the tracer would do with StartSpan and EndSpan
+ */
+
+class TracezProcessorPeer
+{
+public:
+  TracezProcessorPeer(std::shared_ptr<TracezSpanProcessor> &processor_in)
+  {
+    processor  = processor_in;
+  }
+
+  void StartAllSpans(std::vector<std::unique_ptr<ThreadsafeSpanData>> &spans)
+  {
+    for(auto &span : spans)
+    {
+      processor->OnStart(*(span.get()));
+    }
+  }
+
+  void EndAllSpans(std::vector<std::unique_ptr<ThreadsafeSpanData>> &spans)
+  {
+    while(!spans.empty())
+    {
+      processor->OnEnd(std::move(spans.back()));
+      spans.pop_back();
+    }
+  }
+
+private:
+  std::shared_ptr<TracezSpanProcessor> processor;
+};
+
 ////////////////////////  FIXTURE FOR SHARED SETUP CODE ///////////////////
 
 class TracezProcessor : public benchmark::Fixture
@@ -58,13 +86,12 @@ class TracezProcessor : public benchmark::Fixture
   void SetUp(const ::benchmark::State& state)
   {
     processor  = std::shared_ptr<TracezSpanProcessor>(new TracezSpanProcessor());
-    tracer     = std::unique_ptr<opentelemetry::trace::Tracer>(new Tracer(processor));
+    processor_peer = std::unique_ptr<TracezProcessorPeer>(new TracezProcessorPeer(processor));
   }
-
+  std::vector<std::unique_ptr<ThreadsafeSpanData>> spans;
+  std::unique_ptr<TracezProcessorPeer> processor_peer;
   std::shared_ptr<TracezSpanProcessor> processor;
-  std::unique_ptr<opentelemetry::trace::Tracer> tracer;
 
-  std::vector<opentelemetry::nostd::shared_ptr<opentelemetry::trace::Span>> spans;
 };
 
 //////////////////////////// BENCHMARK DEFINITIONS /////////////////////////////////
@@ -78,7 +105,11 @@ BENCHMARK_DEFINE_F(TracezProcessor, BM_MakeRunning)(benchmark::State &state)
   const int num_spans = state.range(0);
   for (auto _ : state)
   {
-    StartManySpans(spans, std::ref(tracer), num_spans);
+    state.PauseTiming();
+    processor_peer->EndAllSpans(spans);
+    CreateRecordables(spans, num_spans);
+    state.ResumeTiming();
+    processor_peer->StartAllSpans(spans);
   }
 }
 
@@ -104,15 +135,18 @@ BENCHMARK_DEFINE_F(TracezProcessor, BM_MakeRunningMakeComplete)(benchmark::State
   const int num_spans = state.range(0);
   for (auto _ : state)
   {
-    std::vector<opentelemetry::nostd::shared_ptr<opentelemetry::trace::Span>> spans2;
-    StartManySpans(spans2, std::ref(tracer), num_spans);
-
-    std::thread start(StartManySpans, std::ref(spans), std::ref(tracer), num_spans);
-    EndAllSpans(spans2);
-
+    state.PauseTiming();
+    processor_peer->EndAllSpans(spans);
+    std::vector<std::unique_ptr<ThreadsafeSpanData>> spans2;
+    CreateRecordables(spans, num_spans);
+    CreateRecordables(spans2, num_spans);
+    processor_peer->StartAllSpans(spans);
+    state.ResumeTiming();
+
+    std::thread start(&TracezProcessorPeer::StartAllSpans, processor_peer.get(), std::ref(spans2));
+    processor_peer->EndAllSpans(spans);
     start.join();
 
-    EndAllSpans(spans);
   }
 }
 
@@ -125,11 +159,15 @@ BENCHMARK_DEFINE_F(TracezProcessor, BM_MakeRunningGetSpans)(benchmark::State &st
   const int num_spans = state.range(0);
   for (auto _ : state)
   {
+    state.PauseTiming();
+    processor_peer->EndAllSpans(spans);
+    CreateRecordables(spans, num_spans);
+    state.ResumeTiming();
+
     std::thread snapshots(GetManySnapshots, std::ref(processor), num_spans);
-    StartManySpans(spans, std::ref(tracer), num_spans);
+    processor_peer->StartAllSpans(spans);
 
     snapshots.join();
-    EndAllSpans(spans);
   }
 }
 
@@ -143,14 +181,16 @@ BENCHMARK_DEFINE_F(TracezProcessor, BM_GetSpansMakeComplete)(benchmark::State &s
   const int num_spans = state.range(0);
   for (auto _ : state)
   {
-    StartManySpans(spans, std::ref(tracer), num_spans);
+    state.PauseTiming();
+    CreateRecordables(spans, num_spans);
+    processor_peer->StartAllSpans(spans);
+    state.ResumeTiming();
 
     std::thread snapshots(GetManySnapshots, std::ref(processor), num_spans);
-    EndAllSpans(spans);
+    processor_peer->EndAllSpans(spans);
 
     snapshots.join();
   }
-
 }
 
 /*
@@ -163,18 +203,20 @@ BENCHMARK_DEFINE_F(TracezProcessor, BM_MakeRunningGetSpansMakeComplete)(benchmar
   const int num_spans = state.range(0);
   for (auto _ : state)
   {
-    std::vector<opentelemetry::nostd::shared_ptr<opentelemetry::trace::Span>> spans2;
+    state.PauseTiming();
+    processor_peer->EndAllSpans(spans);
+    std::vector<std::unique_ptr<ThreadsafeSpanData>> spans2;
+    CreateRecordables(spans, num_spans);
+    CreateRecordables(spans2, num_spans);
+    processor_peer->StartAllSpans(spans);
+    state.ResumeTiming();
 
-    StartManySpans(spans, std::ref(tracer), num_spans);
-
-    std::thread start(StartManySpans, std::ref(spans2), std::ref(tracer), num_spans);
     std::thread snapshots(GetManySnapshots, std::ref(processor), num_spans);
-    EndAllSpans(spans);
-
+    std::thread start(&TracezProcessorPeer::StartAllSpans, processor_peer.get(), std::ref(spans2));
+    processor_peer->EndAllSpans(spans);
     start.join();
     snapshots.join();
 
-    EndAllSpans(spans2);
   }
 }
 
@@ -188,5 +230,11 @@ BENCHMARK_REGISTER_F(TracezProcessor, BM_MakeRunningGetSpans)->Arg(10)->Arg(1000
 BENCHMARK_REGISTER_F(TracezProcessor, BM_GetSpansMakeComplete)->Arg(10)->Arg(1000);
 BENCHMARK_REGISTER_F(TracezProcessor, BM_MakeRunningGetSpansMakeComplete)->Arg(10)->Arg(1000);
 
+} // namespace zpages
+} // namespace ext
+OPENTELEMETRY_END_NAMESPACE
+// setup: create many unique_ptr recordables
+// onstart + onend = 1 vec each (outside of loop)
+// in loop: pause (make recordables, start if onend other) resume. then call peer onstart onend
 BENCHMARK_MAIN();
 

From a89490d929ed33b6bd137bb363f8ff004070aa62 Mon Sep 17 00:00:00 2001
From: Janet Vu <vjanet@umich.edu>
Date: Wed, 26 Aug 2020 14:50:04 +0000
Subject: [PATCH 28/40] Add CPU timers where necessary

---
 .../test/tracez_data_aggregator_benchmark.cc  |  2 +-
 ext/zpages/test/tracez_processor_benchmark.cc | 50 +++++++++++--------
 2 files changed, 29 insertions(+), 23 deletions(-)

diff --git a/ext/zpages/test/tracez_data_aggregator_benchmark.cc b/ext/zpages/test/tracez_data_aggregator_benchmark.cc
index ebf3f450ca..5a42a36f74 100644
--- a/ext/zpages/test/tracez_data_aggregator_benchmark.cc
+++ b/ext/zpages/test/tracez_data_aggregator_benchmark.cc
@@ -311,7 +311,7 @@ BENCHMARK_DEFINE_F(TracezAggregator, BM_ManyBucketsManyNames)(benchmark::State &
   }
 }
 
-//////////////////////// USER VISITS WEBPAGE //////////////////////////////
+////////////////// SAME BENCHMARKS, BUT USER VISITS WEBPAGE ///////////////////////////
 
 BENCHMARK_DEFINE_F(TracezAggregatorFetch, BM_SingleBucketSingleName)(benchmark::State &state)
 {
diff --git a/ext/zpages/test/tracez_processor_benchmark.cc b/ext/zpages/test/tracez_processor_benchmark.cc
index a9e57ce18c..a270acae40 100644
--- a/ext/zpages/test/tracez_processor_benchmark.cc
+++ b/ext/zpages/test/tracez_processor_benchmark.cc
@@ -57,21 +57,25 @@ class TracezProcessorPeer
     processor  = processor_in;
   }
 
+  /*
+   * Calls TracezProcessor::OnStart on each span in spans.
+   */
   void StartAllSpans(std::vector<std::unique_ptr<ThreadsafeSpanData>> &spans)
   {
     for(auto &span : spans)
-    {
       processor->OnStart(*(span.get()));
-    }
   }
 
+  /*
+   * Calls TracezProcessor::OnEnd on each span in spans, which gives ownership
+   * of those spans to the processor. Clears the spans vector since everything in
+   * the spans vector is now garbage.
+   */
   void EndAllSpans(std::vector<std::unique_ptr<ThreadsafeSpanData>> &spans)
   {
-    while(!spans.empty())
-    {
-      processor->OnEnd(std::move(spans.back()));
-      spans.pop_back();
-    }
+    for(auto &span : spans)
+      processor->OnEnd(std::move(span));
+    spans.clear();
   }
 
 private:
@@ -97,8 +101,8 @@ class TracezProcessor : public benchmark::Fixture
 //////////////////////////// BENCHMARK DEFINITIONS /////////////////////////////////
 
 /*
- * Make many empty spans. This checks the scenario where the processor holds
- * many running spans but never gets queried.
+ * Make many empty spans. This checks the scenario where the processor holds many
+ * running spans but never gets queried.
  */
 BENCHMARK_DEFINE_F(TracezProcessor, BM_MakeRunning)(benchmark::State &state)
 {
@@ -109,21 +113,20 @@ BENCHMARK_DEFINE_F(TracezProcessor, BM_MakeRunning)(benchmark::State &state)
     processor_peer->EndAllSpans(spans);
     CreateRecordables(spans, num_spans);
     state.ResumeTiming();
+
     processor_peer->StartAllSpans(spans);
   }
 }
 
 /*
- * Make many snapshots. This checks the scenario where the processor holds
- * no spans but gets queried many times.
+ * Make many snapshots. This checks the scenario where the processor holds no spans
+ * but gets queried many times.
  */
 BENCHMARK_DEFINE_F(TracezProcessor, BM_GetSpans)(benchmark::State &state)
 {
   const int num_spans = state.range(0);
   for (auto _ : state)
-  {
     GetManySnapshots(processor, num_spans);
-  }
 }
 
 /*
@@ -145,8 +148,8 @@ BENCHMARK_DEFINE_F(TracezProcessor, BM_MakeRunningMakeComplete)(benchmark::State
 
     std::thread start(&TracezProcessorPeer::StartAllSpans, processor_peer.get(), std::ref(spans2));
     processor_peer->EndAllSpans(spans);
-    start.join();
 
+    start.join();
   }
 }
 
@@ -214,9 +217,9 @@ BENCHMARK_DEFINE_F(TracezProcessor, BM_MakeRunningGetSpansMakeComplete)(benchmar
     std::thread snapshots(GetManySnapshots, std::ref(processor), num_spans);
     std::thread start(&TracezProcessorPeer::StartAllSpans, processor_peer.get(), std::ref(spans2));
     processor_peer->EndAllSpans(spans);
+
     start.join();
     snapshots.join();
-
   }
 }
 
@@ -225,16 +228,19 @@ BENCHMARK_DEFINE_F(TracezProcessor, BM_MakeRunningGetSpansMakeComplete)(benchmar
 // Arg is the number of spans created for each iteration
 BENCHMARK_REGISTER_F(TracezProcessor, BM_MakeRunning)->Arg(10)->Arg(1000);
 BENCHMARK_REGISTER_F(TracezProcessor, BM_GetSpans)->Arg(10)->Arg(1000);
-BENCHMARK_REGISTER_F(TracezProcessor, BM_MakeRunningMakeComplete)->Arg(10)->Arg(1000);
-BENCHMARK_REGISTER_F(TracezProcessor, BM_MakeRunningGetSpans)->Arg(10)->Arg(1000);
-BENCHMARK_REGISTER_F(TracezProcessor, BM_GetSpansMakeComplete)->Arg(10)->Arg(1000);
-BENCHMARK_REGISTER_F(TracezProcessor, BM_MakeRunningGetSpansMakeComplete)->Arg(10)->Arg(1000);
+// These use multiple threads, so that CPU usage needs to be measured as well
+BENCHMARK_REGISTER_F(TracezProcessor, BM_MakeRunningMakeComplete)->Arg(10)->Arg(1000)
+	->MeasureProcessCPUTime()->UseRealTime();
+BENCHMARK_REGISTER_F(TracezProcessor, BM_MakeRunningGetSpans)->Arg(10)->Arg(1000)
+	->MeasureProcessCPUTime()->UseRealTime();
+BENCHMARK_REGISTER_F(TracezProcessor, BM_GetSpansMakeComplete)->Arg(10)->Arg(1000)
+	->MeasureProcessCPUTime()->UseRealTime();
+BENCHMARK_REGISTER_F(TracezProcessor, BM_MakeRunningGetSpansMakeComplete)->Arg(10)->Arg(1000)
+	->MeasureProcessCPUTime()->UseRealTime();
 
 } // namespace zpages
 } // namespace ext
 OPENTELEMETRY_END_NAMESPACE
-// setup: create many unique_ptr recordables
-// onstart + onend = 1 vec each (outside of loop)
-// in loop: pause (make recordables, start if onend other) resume. then call peer onstart onend
+
 BENCHMARK_MAIN();
 

From 44c809a2d94e89d2632652bb4078ea1f171771f4 Mon Sep 17 00:00:00 2001
From: Janet Vu <vjanet@umich.edu>
Date: Wed, 26 Aug 2020 15:48:31 +0000
Subject: [PATCH 29/40] Add End only benchmark, clear processor running mem fn

---
 ext/zpages/test/tracez_processor_benchmark.cc | 40 +++++++++++++++++--
 1 file changed, 36 insertions(+), 4 deletions(-)

diff --git a/ext/zpages/test/tracez_processor_benchmark.cc b/ext/zpages/test/tracez_processor_benchmark.cc
index a270acae40..55eff7460a 100644
--- a/ext/zpages/test/tracez_processor_benchmark.cc
+++ b/ext/zpages/test/tracez_processor_benchmark.cc
@@ -78,6 +78,15 @@ class TracezProcessorPeer
     spans.clear();
   }
 
+  /*
+   * Clears running span pointers in processor memory, which is used in between
+   * iterations so that spans_.running doesn't hold nullptr
+   */
+  void ClearRunning()
+  {
+    processor->spans_.running.clear();
+  }
+
 private:
   std::shared_ptr<TracezSpanProcessor> processor;
 };
@@ -110,7 +119,11 @@ BENCHMARK_DEFINE_F(TracezProcessor, BM_MakeRunning)(benchmark::State &state)
   for (auto _ : state)
   {
     state.PauseTiming();
-    processor_peer->EndAllSpans(spans);
+    // Clear span vector if EndAllSpans for spans isn't called
+    spans.clear();
+    // Clear processor memory, ensure no nullptrs are in running span set from
+    // previous iterations from either spans or spans2 if EndAllSpans isn't called
+    processor_peer->ClearRunning();
     CreateRecordables(spans, num_spans);
     state.ResumeTiming();
 
@@ -118,6 +131,24 @@ BENCHMARK_DEFINE_F(TracezProcessor, BM_MakeRunning)(benchmark::State &state)
   }
 }
 
+/*
+ * End many empty spans. This checks the scenario where the processor holds many
+ * completed spans but never gets queried.
+ */
+BENCHMARK_DEFINE_F(TracezProcessor, BM_MakeComplete)(benchmark::State &state)
+{
+  const int num_spans = state.range(0);
+  for (auto _ : state)
+  {
+    state.PauseTiming();
+    CreateRecordables(spans, num_spans);
+    processor_peer->StartAllSpans(spans);
+    state.ResumeTiming();
+
+    processor_peer->EndAllSpans(spans);
+  }
+}
+
 /*
  * Make many snapshots. This checks the scenario where the processor holds no spans
  * but gets queried many times.
@@ -139,7 +170,7 @@ BENCHMARK_DEFINE_F(TracezProcessor, BM_MakeRunningMakeComplete)(benchmark::State
   for (auto _ : state)
   {
     state.PauseTiming();
-    processor_peer->EndAllSpans(spans);
+    processor_peer->ClearRunning();
     std::vector<std::unique_ptr<ThreadsafeSpanData>> spans2;
     CreateRecordables(spans, num_spans);
     CreateRecordables(spans2, num_spans);
@@ -163,7 +194,8 @@ BENCHMARK_DEFINE_F(TracezProcessor, BM_MakeRunningGetSpans)(benchmark::State &st
   for (auto _ : state)
   {
     state.PauseTiming();
-    processor_peer->EndAllSpans(spans);
+    spans.clear();
+    processor_peer->ClearRunning();
     CreateRecordables(spans, num_spans);
     state.ResumeTiming();
 
@@ -207,7 +239,7 @@ BENCHMARK_DEFINE_F(TracezProcessor, BM_MakeRunningGetSpansMakeComplete)(benchmar
   for (auto _ : state)
   {
     state.PauseTiming();
-    processor_peer->EndAllSpans(spans);
+    processor_peer->ClearRunning();
     std::vector<std::unique_ptr<ThreadsafeSpanData>> spans2;
     CreateRecordables(spans, num_spans);
     CreateRecordables(spans2, num_spans);

From 793c91b989e183767c401154a8cb002ace16c5bc Mon Sep 17 00:00:00 2001
From: Janet Vu <vjanet@umich.edu>
Date: Wed, 26 Aug 2020 15:54:00 +0000
Subject: [PATCH 30/40] Rename i to num_spans

---
 .../test/tracez_data_aggregator_benchmark.cc  | 41 ++++++++++---------
 ext/zpages/test/tracez_processor_benchmark.cc | 12 +++---
 2 files changed, 27 insertions(+), 26 deletions(-)

diff --git a/ext/zpages/test/tracez_data_aggregator_benchmark.cc b/ext/zpages/test/tracez_data_aggregator_benchmark.cc
index 5a42a36f74..2f067e60b2 100644
--- a/ext/zpages/test/tracez_data_aggregator_benchmark.cc
+++ b/ext/zpages/test/tracez_data_aggregator_benchmark.cc
@@ -19,56 +19,57 @@ namespace zpages
 /////////////////////////////// BENCHMARK HELPER FUNCTIONS //////////////////////////////
 
 /*
- * Helper function that creates and ends i spans instantly. If is_unique is
+ * Helper function that creates and ends num_spans spans instantly. If is_unique is
  * true, then all spans will have different names.
  */
 void StartEndSpans(
     std::shared_ptr<opentelemetry::trace::Tracer> &tracer,
-    int i,
+    int num_spans,
     bool is_unique = false)
 {
   opentelemetry::trace::StartSpanOptions start;
   opentelemetry::trace::EndSpanOptions end;
   start.start_steady_time = SteadyTimestamp(nanoseconds(0));
   end.end_steady_time = SteadyTimestamp(nanoseconds(1));
-  for (; i > 0; i--)
-    tracer->StartSpan(is_unique ? std::to_string(i) : "", start)->End(end);
+  for (; num_spans > 0; num_spans--)
+    tracer->StartSpan(is_unique ? std::to_string(num_spans) : "", start)->End(end);
 }
 
 /*
- * Helper function that creates and ends spans instantly, while simulating
- * different latencies. If is_unique is true, then all spans will have
- * different names.
+ * Helper function that creates and ends num_spans spans instantly, while
+ * simulating different latencies. If is_unique is true, then all spans will
+ * have different names.
  */
 void StartEndSpansLatency(
     std::shared_ptr<opentelemetry::trace::Tracer> &tracer,
-    int i,
+    int num_spans,
     bool is_unique = false)
 {
   opentelemetry::trace::StartSpanOptions start;
   start.start_steady_time = SteadyTimestamp(nanoseconds(0));
-  for (; i > 0; i--)
+  for (; num_spans > 0; num_spans--)
   {
     // Latency bucket depends on the index
-    auto latency_band = kLatencyBoundaries[i % kLatencyBoundaries.size()];
+    auto latency_band = kLatencyBoundaries[num_spans % kLatencyBoundaries.size()];
     opentelemetry::trace::EndSpanOptions end;
     end.end_steady_time = SteadyTimestamp(latency_band);
 
-    tracer->StartSpan(is_unique ? std::to_string(i) : "", start)->End(end);
+    tracer->StartSpan(is_unique ? std::to_string(num_spans) : "", start)->End(end);
   }
 }
 
 /*
- * Helper function that creates and ends spans instantly, while simulating
- * error codes. If is_unique is true, then all spans will have different names.
+ * Helper function that creates and ends num_spans spans instantly, while
+ * simulating error codes. If is_unique is true, then all spans will have
+ * different names.
  */
 void StartEndSpansError(
     std::shared_ptr<opentelemetry::trace::Tracer> &tracer,
-    int i,
+    int num_spans,
     bool is_unique = false)
 {
-  for (; i > 0; i--)
-    tracer->StartSpan(is_unique ? std::to_string(i) : "")
+  for (; num_spans > 0; num_spans--)
+    tracer->StartSpan(is_unique ? std::to_string(num_spans) : "")
         ->SetStatus(opentelemetry::trace::CanonicalCode::CANCELLED, "");
 }
 
@@ -80,17 +81,17 @@ void StartEndSpansError(
 void StartSpans(
     std::vector<opentelemetry::nostd::shared_ptr<opentelemetry::trace::Span>> &spans,
     std::shared_ptr<opentelemetry::trace::Tracer> &tracer,
-    int i,
+    int num_spans,
     bool is_unique = false)
 {
-  for (; i > 0; i--)
-    spans.push_back(tracer->StartSpan(is_unique ? std::to_string(i) : ""));
+  for (; num_spans > 0; num_spans--)
+    spans.push_back(tracer->StartSpan(is_unique ? std::to_string(num_spans) : ""));
 }
 
 /*
  * Helper function that creates about i spans, evenly split between running, latency (which
  * is further split by bucket), and error. If is_unique is true, then all spans will have
- * different names.
+ * different names. The running spans vector is returned so that they remain running.
  */
 std::vector<opentelemetry::nostd::shared_ptr<opentelemetry::trace::Span>> MakeManySpans(
     std::shared_ptr<opentelemetry::trace::Tracer> &tracer,
diff --git a/ext/zpages/test/tracez_processor_benchmark.cc b/ext/zpages/test/tracez_processor_benchmark.cc
index 55eff7460a..91a2cc82cb 100644
--- a/ext/zpages/test/tracez_processor_benchmark.cc
+++ b/ext/zpages/test/tracez_processor_benchmark.cc
@@ -16,27 +16,27 @@ namespace zpages
 /////////////////////////////// BENCHMARK HELPER FUNCTIONS //////////////////////////////
 
 /*
- * Helper function that creates i threadsafe span data, adding them to the spans
+ * Helper function that creates num_spans threadsafe span data, adding them to the spans
  * vector during setup to benchmark OnStart and OnEnd times for the processor.
  */
 void CreateRecordables(
     std::vector<std::unique_ptr<ThreadsafeSpanData>> &spans,
-    int i)
+    int num_spans)
 {
-  for (; i > 0; i--)
+  for (; num_spans > 0; num_spans--)
     spans.push_back(std::unique_ptr<ThreadsafeSpanData>(new ThreadsafeSpanData()));
 }
 
 /*
- * Helper function calls GetSpanSnapshot() i times, does nothing otherwise.
+ * Helper function calls GetSpanSnapshot() num_snapshots times, does nothing otherwise.
  * Snapshots are significant and contribute to performance differences because
  * completed spans are cleared from the processor memory. This function
  * simulates an aggregator querying the processor many times, but doesn't
  * aggregate or store spans.
  */
-void GetManySnapshots(std::shared_ptr<TracezSpanProcessor> &processor, int i)
+void GetManySnapshots(std::shared_ptr<TracezSpanProcessor> &processor, int num_snapshots)
 {
-  for (; i > 0; i--)
+  for (; num_snapshots > 0; num_snapshots--)
     processor->GetSpanSnapshot();
 }
 

From a8febb31e02300a0055fd22bdcc4971bc37670d1 Mon Sep 17 00:00:00 2001
From: Janet Vu <vjanet@umich.edu>
Date: Wed, 26 Aug 2020 15:57:29 +0000
Subject: [PATCH 31/40] Name periodic function things better

---
 .../test/tracez_data_aggregator_benchmark.cc    | 17 ++++++++---------
 1 file changed, 8 insertions(+), 9 deletions(-)

diff --git a/ext/zpages/test/tracez_data_aggregator_benchmark.cc b/ext/zpages/test/tracez_data_aggregator_benchmark.cc
index 2f067e60b2..5f4859ea73 100644
--- a/ext/zpages/test/tracez_data_aggregator_benchmark.cc
+++ b/ext/zpages/test/tracez_data_aggregator_benchmark.cc
@@ -158,17 +158,16 @@ class TracezDataAggregatorPeer
   }
 
   /*
-   * Starts a background thread to query for aggregated data every i
-   * nanoseconds. This should affect performance for the aggregator,
-   * since aggregation work cannot be done while aggegation data is being
-   * requested. This simulates a user visiting or refreshing the Tracez
-   * webpage many times, except this function does and holds nothing in
-   * memory.
+   * Starts a background thread to query for aggregated data every
+   * query_interval nanoseconds. This should affect performance for the
+   * aggregator, since aggregation work cannot be done while aggegation
+   * data is being requested. This simulates a user visiting or refreshing
+   * the Tracez webpage many times, except this function does and holds
+   * nothing in memory.
    */
-  void StartPeriodicQuery(int i = 1)
+  void StartPeriodicQueryThread(nanoseconds query_interval = nanoseconds(1))
   {
     run.store(true, std::memory_order_release);
-    nanoseconds query_interval = nanoseconds(i);
     query_thread = std::thread([this, query_interval]() {
       while (run.load(std::memory_order_acquire))
       {
@@ -229,7 +228,7 @@ class TracezAggregatorFetch : public TracezAggregator
   void SetUp(const ::benchmark::State& state) override
   {
     SetTracerAggregatorPeer();
-    aggregator_peer->StartPeriodicQuery();
+    aggregator_peer->StartPeriodicQueryThread();
   }
 };
 

From a9c2619593a8915edf43c47777b0f512f0f2e252 Mon Sep 17 00:00:00 2001
From: Janet Vu <vjanet@umich.edu>
Date: Wed, 26 Aug 2020 16:08:23 +0000
Subject: [PATCH 32/40] Reorder functions in benchmarks

---
 ext/zpages/test/tracez_processor_benchmark.cc | 18 +++++++++---------
 1 file changed, 9 insertions(+), 9 deletions(-)

diff --git a/ext/zpages/test/tracez_processor_benchmark.cc b/ext/zpages/test/tracez_processor_benchmark.cc
index 91a2cc82cb..78a4a3923d 100644
--- a/ext/zpages/test/tracez_processor_benchmark.cc
+++ b/ext/zpages/test/tracez_processor_benchmark.cc
@@ -199,10 +199,10 @@ BENCHMARK_DEFINE_F(TracezProcessor, BM_MakeRunningGetSpans)(benchmark::State &st
     CreateRecordables(spans, num_spans);
     state.ResumeTiming();
 
-    std::thread snapshots(GetManySnapshots, std::ref(processor), num_spans);
-    processor_peer->StartAllSpans(spans);
+    std::thread start(&TracezProcessorPeer::StartAllSpans, processor_peer.get(), std::ref(spans));
+    GetManySnapshots(processor, num_spans);
 
-    snapshots.join();
+    start.join();
   }
 }
 
@@ -221,10 +221,10 @@ BENCHMARK_DEFINE_F(TracezProcessor, BM_GetSpansMakeComplete)(benchmark::State &s
     processor_peer->StartAllSpans(spans);
     state.ResumeTiming();
 
-    std::thread snapshots(GetManySnapshots, std::ref(processor), num_spans);
-    processor_peer->EndAllSpans(spans);
+    std::thread end(&TracezProcessorPeer::EndAllSpans, processor_peer.get(), std::ref(spans));
+    GetManySnapshots(processor, num_spans);
 
-    snapshots.join();
+    end.join();
   }
 }
 
@@ -246,12 +246,12 @@ BENCHMARK_DEFINE_F(TracezProcessor, BM_MakeRunningGetSpansMakeComplete)(benchmar
     processor_peer->StartAllSpans(spans);
     state.ResumeTiming();
 
-    std::thread snapshots(GetManySnapshots, std::ref(processor), num_spans);
+    std::thread end(&TracezProcessorPeer::EndAllSpans, processor_peer.get(), std::ref(spans));
     std::thread start(&TracezProcessorPeer::StartAllSpans, processor_peer.get(), std::ref(spans2));
-    processor_peer->EndAllSpans(spans);
+    GetManySnapshots(processor, num_spans);
 
     start.join();
-    snapshots.join();
+    end.join();
   }
 }
 

From de5744353a6e8b52c0b5704513960c095486b123 Mon Sep 17 00:00:00 2001
From: Janet Vu <vjanet@umich.edu>
Date: Wed, 26 Aug 2020 16:50:06 +0000
Subject: [PATCH 33/40] Cleanup

---
 ext/zpages/test/tracez_data_aggregator_benchmark.cc | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/ext/zpages/test/tracez_data_aggregator_benchmark.cc b/ext/zpages/test/tracez_data_aggregator_benchmark.cc
index 5f4859ea73..d22aa6efa9 100644
--- a/ext/zpages/test/tracez_data_aggregator_benchmark.cc
+++ b/ext/zpages/test/tracez_data_aggregator_benchmark.cc
@@ -172,8 +172,8 @@ class TracezDataAggregatorPeer
       while (run.load(std::memory_order_acquire))
       {
         std::unique_lock<std::mutex> lock(mtx);
-        aggregator->GetAggregatedTracezData();
-        // Continue if mutex is free and query interval passed
+        auto aggregations = aggregator->GetAggregatedTracezData();
+        // Continue if query interval passed
         cont.wait_for(lock, query_interval);
       }
     });
@@ -212,7 +212,6 @@ class TracezAggregator : public benchmark::Fixture
     SetTracerAggregatorPeer();
   }
 
-  // Aggregator peer for accessing private aggregator functions
   std::unique_ptr<TracezDataAggregatorPeer> aggregator_peer;
   // Tracer for creating spans
   std::shared_ptr<opentelemetry::trace::Tracer> tracer;

From 5fc728fe3e5c04db0188dd058b8649a45fb00744 Mon Sep 17 00:00:00 2001
From: Janet Vu <vjanet@umich.edu>
Date: Thu, 27 Aug 2020 00:13:31 +0000
Subject: [PATCH 34/40] Deduplicate aggregator benchmark code

---
 .../test/tracez_data_aggregator_benchmark.cc  | 93 ++++++++++---------
 1 file changed, 47 insertions(+), 46 deletions(-)

diff --git a/ext/zpages/test/tracez_data_aggregator_benchmark.cc b/ext/zpages/test/tracez_data_aggregator_benchmark.cc
index d22aa6efa9..b08386ff9f 100644
--- a/ext/zpages/test/tracez_data_aggregator_benchmark.cc
+++ b/ext/zpages/test/tracez_data_aggregator_benchmark.cc
@@ -231,9 +231,8 @@ class TracezAggregatorFetch : public TracezAggregator
   }
 };
 
-//////////////////////////// BENCHMARK DEFINITIONS /////////////////////////////////
 
-//////////////////////// USER NEVER VISITS WEBPAGE //////////////////////////////
+////////////////////////// BENCHMARK HELPERS ///////////////////////////////
 
 /*
  * Aggregator handing many spans with the same name, who end instantly. This
@@ -241,14 +240,16 @@ class TracezAggregatorFetch : public TracezAggregator
  * of latencies is required, as all spans should be sorted in the same bucket
  * under the same span name.
  */
-BENCHMARK_DEFINE_F(TracezAggregator, BM_SingleBucketSingleName)(benchmark::State &state)
+void SingleBucketSingleName(
+    benchmark::State &state,
+    std::shared_ptr<opentelemetry::trace::Tracer> &tracer,
+    std::unique_ptr<TracezDataAggregatorPeer> const &aggregator_peer)
 {
-  const int num_spans = state.range(0);
   for (auto _ : state)
   {
     // Do not time span creation, as we're only benchmarking aggregation work
     state.PauseTiming();
-    StartEndSpans(tracer, num_spans);
+    StartEndSpans(tracer, state.range(0));
     state.ResumeTiming();
     aggregator_peer->Aggregate();
   }
@@ -260,13 +261,15 @@ BENCHMARK_DEFINE_F(TracezAggregator, BM_SingleBucketSingleName)(benchmark::State
  * of latencies is required. Spans are sorted in different groups but always
  * in the same bucket.
  */
-BENCHMARK_DEFINE_F(TracezAggregator, BM_SingleBucketManyNames)(benchmark::State &state)
+void SingleBucketManyNames(
+    benchmark::State &state,
+    std::shared_ptr<opentelemetry::trace::Tracer> &tracer,
+    std::unique_ptr<TracezDataAggregatorPeer> const &aggregator_peer)
 {
-  const int num_spans = state.range(0);
   for (auto _ : state)
   {
     state.PauseTiming();
-    StartEndSpans(tracer, num_spans, false);
+    StartEndSpans(tracer, state.range(0), true);
     state.ResumeTiming();
     aggregator_peer->Aggregate();
   }
@@ -279,13 +282,15 @@ BENCHMARK_DEFINE_F(TracezAggregator, BM_SingleBucketManyNames)(benchmark::State
  * respective buckets. Spans are in the same group but sorted to different
  * buckets.
  */
-BENCHMARK_DEFINE_F(TracezAggregator, BM_ManyBucketsSingleName)(benchmark::State &state)
+void ManyBucketsSingleName(
+    benchmark::State &state,
+    std::shared_ptr<opentelemetry::trace::Tracer> &tracer,
+    std::unique_ptr<TracezDataAggregatorPeer> const &aggregator_peer)
 {
-  const int num_spans = state.range(0);
   for (auto _ : state)
   {
     state.PauseTiming();
-    auto running_spans = MakeManySpans(tracer, num_spans, false);
+    auto running_spans = MakeManySpans(tracer, state.range(0));
     state.ResumeTiming();
     aggregator_peer->Aggregate();
   }
@@ -298,66 +303,62 @@ BENCHMARK_DEFINE_F(TracezAggregator, BM_ManyBucketsSingleName)(benchmark::State
  * respective buckets. Spans are in the different groups and buckets, similar to
  * likely real use cases.
  */
-BENCHMARK_DEFINE_F(TracezAggregator, BM_ManyBucketsManyNames)(benchmark::State &state)
+void ManyBucketsManyNames(
+    benchmark::State &state,
+    std::shared_ptr<opentelemetry::trace::Tracer> &tracer,
+    std::unique_ptr<TracezDataAggregatorPeer> const &aggregator_peer)
 {
-  const int num_spans = state.range(0);
   for (auto _ : state)
   {
     state.PauseTiming();
-    auto running_spans = MakeManySpans(tracer, num_spans, true);
+    auto running_spans = MakeManySpans(tracer, state.range(0));
     state.ResumeTiming();
     aggregator_peer->Aggregate();
   }
 }
 
+//////////////////////// USER NEVER VISITS WEBPAGE //////////////////////////////
+
+BENCHMARK_DEFINE_F(TracezAggregator, BM_SingleBucketSingleName)(benchmark::State &state)
+{
+  SingleBucketSingleName(state, tracer, aggregator_peer);
+}
+
+BENCHMARK_DEFINE_F(TracezAggregator, BM_SingleBucketManyNames)(benchmark::State &state)
+{
+  SingleBucketManyNames(state, tracer, aggregator_peer);
+}
+
+BENCHMARK_DEFINE_F(TracezAggregator, BM_ManyBucketsSingleName)(benchmark::State &state)
+{
+  ManyBucketsSingleName(state, tracer, aggregator_peer);
+}
+
+BENCHMARK_DEFINE_F(TracezAggregator, BM_ManyBucketsManyNames)(benchmark::State &state)
+{
+  ManyBucketsManyNames(state, tracer, aggregator_peer);
+}
+
 ////////////////// SAME BENCHMARKS, BUT USER VISITS WEBPAGE ///////////////////////////
 
 BENCHMARK_DEFINE_F(TracezAggregatorFetch, BM_SingleBucketSingleName)(benchmark::State &state)
 {
-  const int num_spans = state.range(0);
-  for (auto _ : state)
-  {
-    state.PauseTiming();
-    StartEndSpans(tracer, num_spans);
-    state.ResumeTiming();
-    aggregator_peer->Aggregate();
-  }
+  SingleBucketSingleName(state, tracer, aggregator_peer);
 }
 
 BENCHMARK_DEFINE_F(TracezAggregatorFetch, BM_SingleBucketManyNames)(benchmark::State &state)
 {
-  const int num_spans = state.range(0);
-  for (auto _ : state)
-  {
-    state.PauseTiming();
-    StartEndSpans(tracer, num_spans, false);
-    state.ResumeTiming();
-    aggregator_peer->Aggregate();
-  }
+  SingleBucketManyNames(state, tracer, aggregator_peer);
 }
 
 BENCHMARK_DEFINE_F(TracezAggregatorFetch, BM_ManyBucketsSingleName)(benchmark::State &state)
 {
-  const int num_spans = state.range(0);
-  for (auto _ : state)
-  {
-    state.PauseTiming();
-    auto running_spans = MakeManySpans(tracer, num_spans);
-    state.ResumeTiming();
-    aggregator_peer->Aggregate();
-  }
+  ManyBucketsSingleName(state, tracer, aggregator_peer);
 }
 
 BENCHMARK_DEFINE_F(TracezAggregatorFetch, BM_ManyBucketsManyNames)(benchmark::State &state)
 {
-  const int num_spans = state.range(0);
-  for (auto _ : state)
-  {
-    state.PauseTiming();
-    auto running_spans = MakeManySpans(tracer, num_spans, true);
-    state.ResumeTiming();
-    aggregator_peer->Aggregate();
-  }
+  ManyBucketsManyNames(state, tracer, aggregator_peer);
 }
 
 //////////////////////////// RUN BENCHMARKS ///////////////////////////////

From 4b16f829964db243a2adb934b6bd8afd26c83c36 Mon Sep 17 00:00:00 2001
From: Janet Vu <vjanet@umich.edu>
Date: Thu, 27 Aug 2020 13:28:57 +0000
Subject: [PATCH 35/40] Cosmetic changes from code review

---
 .../test/tracez_data_aggregator_benchmark.cc  | 148 ++++++++----------
 ext/zpages/test/tracez_processor_benchmark.cc |  82 +++++-----
 2 files changed, 107 insertions(+), 123 deletions(-)

diff --git a/ext/zpages/test/tracez_data_aggregator_benchmark.cc b/ext/zpages/test/tracez_data_aggregator_benchmark.cc
index b08386ff9f..9709d8379e 100644
--- a/ext/zpages/test/tracez_data_aggregator_benchmark.cc
+++ b/ext/zpages/test/tracez_data_aggregator_benchmark.cc
@@ -36,9 +36,9 @@ void StartEndSpans(
 }
 
 /*
- * Helper function that creates and ends num_spans spans instantly, while
- * simulating different latencies. If is_unique is true, then all spans will
- * have different names.
+ * Helper function that creates and ends num_spans spans instantly, while evenly
+ * spreading spans across all latency bands. If is_unique is true, then all spans
+ * will have different names.
  */
 void StartEndSpansLatency(
     std::shared_ptr<opentelemetry::trace::Tracer> &tracer,
@@ -50,7 +50,7 @@ void StartEndSpansLatency(
   for (; num_spans > 0; num_spans--)
   {
     // Latency bucket depends on the index
-    auto latency_band = kLatencyBoundaries[num_spans % kLatencyBoundaries.size()];
+    nanoseconds latency_band = kLatencyBoundaries[num_spans % kLatencyBoundaries.size()];
     opentelemetry::trace::EndSpanOptions end;
     end.end_steady_time = SteadyTimestamp(latency_band);
 
@@ -74,7 +74,7 @@ void StartEndSpansError(
 }
 
 /*
- * Helper function that creates i spans that always run. To remain in scope and running,
+ * Helper function that creates num_spans spans that always run. To remain in scope and running,
  * spans are referenced in the passed in vector. If is_unique is true, then all spans
  * will have different names.
  */
@@ -89,7 +89,7 @@ void StartSpans(
 }
 
 /*
- * Helper function that creates about i spans, evenly split between running, latency (which
+ * Helper function that creates about num_spans spans, evenly split between running, latency (which
  * is further split by bucket), and error. If is_unique is true, then all spans will have
  * different names. The running spans vector is returned so that they remain running.
  */
@@ -122,17 +122,17 @@ class TracezDataAggregatorPeer
   TracezDataAggregatorPeer(std::shared_ptr<TracezSpanProcessor> processor)
   {
     // Set up the aggregator
-    aggregator = std::unique_ptr<TracezDataAggregator>(
+    aggregator_ = std::unique_ptr<TracezDataAggregator>(
         new TracezDataAggregator(processor));
 
     // Disable the aggregetor's periodic background thread aggregation work, which
     // it normally does during production. Disabling it allows us to isolate
     // aggregation work for benchmarking
-    if (aggregator->execute_.load(std::memory_order_acquire))
+    if (aggregator_->execute_.load(std::memory_order_acquire))
     {
-      aggregator->execute_.store(false, std::memory_order_release);
-      aggregator->cv_.notify_one();
-      aggregator->aggregate_spans_thread_.join();
+      aggregator_->execute_.store(false, std::memory_order_release);
+      aggregator_->cv_.notify_one();
+      aggregator_->aggregate_spans_thread_.join();
     }
   }
 
@@ -141,10 +141,10 @@ class TracezDataAggregatorPeer
    */
   ~TracezDataAggregatorPeer()
   {
-    if (run.load(std::memory_order_acquire))
+    if (run_.load(std::memory_order_acquire))
     {
-      run.store(false, std::memory_order_release);
-      query_thread.join();
+      run_.store(false, std::memory_order_release);
+      query_thread_.join();
     }
   }
 
@@ -153,8 +153,8 @@ class TracezDataAggregatorPeer
    */
   void Aggregate()
   {
-    std::lock_guard<std::mutex> lock(mtx);
-    aggregator->AggregateSpans();
+    std::lock_guard<std::mutex> lock(mtx_);
+    aggregator_->AggregateSpans();
   }
 
   /*
@@ -167,26 +167,26 @@ class TracezDataAggregatorPeer
    */
   void StartPeriodicQueryThread(nanoseconds query_interval = nanoseconds(1))
   {
-    run.store(true, std::memory_order_release);
-    query_thread = std::thread([this, query_interval]() {
-      while (run.load(std::memory_order_acquire))
+    run_.store(true, std::memory_order_release);
+    query_thread_ = std::thread([this, query_interval]() {
+      while (run_.load(std::memory_order_acquire))
       {
-        std::unique_lock<std::mutex> lock(mtx);
-        auto aggregations = aggregator->GetAggregatedTracezData();
+        std::unique_lock<std::mutex> lock(mtx_);
+        auto aggregations = aggregator_->GetAggregatedTracezData();
         // Continue if query interval passed
-        cont.wait_for(lock, query_interval);
+        cont_.wait_for(lock, query_interval);
       }
     });
   }
 
 private:
-  std::unique_ptr<TracezDataAggregator> aggregator;
+  std::unique_ptr<TracezDataAggregator> aggregator_;
   // Keep queries running while peer is in memory
-  std::thread query_thread;
+  std::thread query_thread_;
   // Ensure queries and aggregations don't happen simultaneously
-  std::mutex mtx;
-  std::atomic<bool> run;
-  std::condition_variable cont;
+  std::mutex mtx_;
+  std::atomic<bool> run_;
+  std::condition_variable cont_;
   
 };
 
@@ -199,46 +199,26 @@ class TracezDataAggregatorPeer
 class TracezAggregator : public benchmark::Fixture
 {
 protected:
-  void SetTracerAggregatorPeer()
+  void SetUp(const ::benchmark::State& state)
   {
     std::shared_ptr<TracezSpanProcessor> processor(new TracezSpanProcessor());
-    tracer = std::shared_ptr<opentelemetry::trace::Tracer>(new Tracer(processor));
-    aggregator_peer = std::unique_ptr<TracezDataAggregatorPeer>(
+    tracer_ = std::shared_ptr<opentelemetry::trace::Tracer>(new Tracer(processor));
+    aggregator_peer_ = std::unique_ptr<TracezDataAggregatorPeer>(
         new TracezDataAggregatorPeer(processor));
   }
 
-  void SetUp(const ::benchmark::State& state)
-  {
-    SetTracerAggregatorPeer();
-  }
-
-  std::unique_ptr<TracezDataAggregatorPeer> aggregator_peer;
+  std::unique_ptr<TracezDataAggregatorPeer> aggregator_peer_;
   // Tracer for creating spans
-  std::shared_ptr<opentelemetry::trace::Tracer> tracer;
-};
-
-/*
- * This extend the previous fixture, adding in the periodic query work in
- * the background to simulate users requesting the data on the frontend
- */
-class TracezAggregatorFetch : public TracezAggregator
-{
-protected:
-  void SetUp(const ::benchmark::State& state) override
-  {
-    SetTracerAggregatorPeer();
-    aggregator_peer->StartPeriodicQueryThread();
-  }
+  std::shared_ptr<opentelemetry::trace::Tracer> tracer_;
 };
 
-
 ////////////////////////// BENCHMARK HELPERS ///////////////////////////////
 
 /*
- * Aggregator handing many spans with the same name, who end instantly. This
+ * Aggregator handling many spans with the same name, who end instantly. This
  * checks the scenario where there's only one Tracez name and minimal sorting
- * of latencies is required, as all spans should be sorted in the same bucket
- * under the same span name.
+ * of spans into different latency bands is required, as all spans should be
+ * sorted in the same bucket under the same span name.
  */
 void SingleBucketSingleName(
     benchmark::State &state,
@@ -256,10 +236,10 @@ void SingleBucketSingleName(
 }
 
 /*
- * Aggregator handing many spans with unique names, who end instantly. This
+ * Aggregator handling many spans with unique names, who end instantly. This
  * checks the scenario where there's many Tracez groups but minimal sorting
- * of latencies is required. Spans are sorted in different groups but always
- * in the same bucket.
+ * of spans into different latency bands is required. Spans are sorted in
+ * different groups but always in the same bucket.
  */
 void SingleBucketManyNames(
     benchmark::State &state,
@@ -276,11 +256,11 @@ void SingleBucketManyNames(
 }
 
 /*
- * Aggregator handing many spans with the same name, who may fall under error,
+ * Aggregator handling many spans with the same name, who may fall under error,
  * running, and any latency group. This checks the scenario where there's only
  * one Tracez groups but there also needs to be sorting of spans into their
- * respective buckets. Spans are in the same group but sorted to different
- * buckets.
+ * respective buckets (running, error, latency [including within latency bands]).
+ * Spans are in the same group but sorted to different buckets.
  */
 void ManyBucketsSingleName(
     benchmark::State &state,
@@ -297,11 +277,11 @@ void ManyBucketsSingleName(
 }
 
 /*
- * Aggregator handing many spans with unique names, who may fall under error,
+ * Aggregator handling many spans with unique names, who may fall under error,
  * running, and any latency group. This checks the scenario where there's many
- * Tracez groups and there needs to be sorting of spans into their
- * respective buckets. Spans are in the different groups and buckets, similar to
- * likely real use cases.
+ * Tracez groups and there also needs to be sorting of spans into their
+ * respective buckets (running, error, latency [including within latency bands]).
+ * Spans are in the same group but sorted to different buckets.
  */
 void ManyBucketsManyNames(
     benchmark::State &state,
@@ -321,57 +301,61 @@ void ManyBucketsManyNames(
 
 BENCHMARK_DEFINE_F(TracezAggregator, BM_SingleBucketSingleName)(benchmark::State &state)
 {
-  SingleBucketSingleName(state, tracer, aggregator_peer);
+  SingleBucketSingleName(state, tracer_, aggregator_peer_);
 }
 
 BENCHMARK_DEFINE_F(TracezAggregator, BM_SingleBucketManyNames)(benchmark::State &state)
 {
-  SingleBucketManyNames(state, tracer, aggregator_peer);
+  SingleBucketManyNames(state, tracer_, aggregator_peer_);
 }
 
 BENCHMARK_DEFINE_F(TracezAggregator, BM_ManyBucketsSingleName)(benchmark::State &state)
 {
-  ManyBucketsSingleName(state, tracer, aggregator_peer);
+  ManyBucketsSingleName(state, tracer_, aggregator_peer_);
 }
 
 BENCHMARK_DEFINE_F(TracezAggregator, BM_ManyBucketsManyNames)(benchmark::State &state)
 {
-  ManyBucketsManyNames(state, tracer, aggregator_peer);
+  ManyBucketsManyNames(state, tracer_, aggregator_peer_);
 }
 
-////////////////// SAME BENCHMARKS, BUT USER VISITS WEBPAGE ///////////////////////////
+///////////// SAME BENCHMARKS, BUT AGGREGATIONS REQUESTED PERIODICALLY /////////////////
 
-BENCHMARK_DEFINE_F(TracezAggregatorFetch, BM_SingleBucketSingleName)(benchmark::State &state)
+BENCHMARK_DEFINE_F(TracezAggregator, BM_SingleBucketSingleNameFetch)(benchmark::State &state)
 {
-  SingleBucketSingleName(state, tracer, aggregator_peer);
+  aggregator_peer_->StartPeriodicQueryThread();
+  SingleBucketSingleName(state, tracer_, aggregator_peer_);
 }
 
-BENCHMARK_DEFINE_F(TracezAggregatorFetch, BM_SingleBucketManyNames)(benchmark::State &state)
+BENCHMARK_DEFINE_F(TracezAggregator, BM_SingleBucketManyNamesFetch)(benchmark::State &state)
 {
-  SingleBucketManyNames(state, tracer, aggregator_peer);
+  aggregator_peer_->StartPeriodicQueryThread();
+  SingleBucketManyNames(state, tracer_, aggregator_peer_);
 }
 
-BENCHMARK_DEFINE_F(TracezAggregatorFetch, BM_ManyBucketsSingleName)(benchmark::State &state)
+BENCHMARK_DEFINE_F(TracezAggregator, BM_ManyBucketsSingleNameFetch)(benchmark::State &state)
 {
-  ManyBucketsSingleName(state, tracer, aggregator_peer);
+  aggregator_peer_->StartPeriodicQueryThread();
+  ManyBucketsSingleName(state, tracer_, aggregator_peer_);
 }
 
-BENCHMARK_DEFINE_F(TracezAggregatorFetch, BM_ManyBucketsManyNames)(benchmark::State &state)
+BENCHMARK_DEFINE_F(TracezAggregator, BM_ManyBucketsManyNamesFetch)(benchmark::State &state)
 {
-  ManyBucketsManyNames(state, tracer, aggregator_peer);
+  aggregator_peer_->StartPeriodicQueryThread();
+  ManyBucketsManyNames(state, tracer_, aggregator_peer_);
 }
 
 //////////////////////////// RUN BENCHMARKS ///////////////////////////////
 
 // Arg is the number of spans created for each iteration
 BENCHMARK_REGISTER_F(TracezAggregator, BM_SingleBucketSingleName)->Arg(10)->Arg(1000);
-BENCHMARK_REGISTER_F(TracezAggregatorFetch, BM_SingleBucketSingleName)->Arg(10)->Arg(1000);
+BENCHMARK_REGISTER_F(TracezAggregator, BM_SingleBucketSingleNameFetch)->Arg(10)->Arg(1000);
 BENCHMARK_REGISTER_F(TracezAggregator, BM_SingleBucketManyNames)->Arg(10)->Arg(1000);
-BENCHMARK_REGISTER_F(TracezAggregatorFetch, BM_SingleBucketManyNames)->Arg(10)->Arg(1000);
+BENCHMARK_REGISTER_F(TracezAggregator, BM_SingleBucketManyNamesFetch)->Arg(10)->Arg(1000);
 BENCHMARK_REGISTER_F(TracezAggregator, BM_ManyBucketsSingleName)->Arg(10)->Arg(1000);
-BENCHMARK_REGISTER_F(TracezAggregatorFetch, BM_ManyBucketsSingleName)->Arg(10)->Arg(1000);
+BENCHMARK_REGISTER_F(TracezAggregator, BM_ManyBucketsSingleNameFetch)->Arg(10)->Arg(1000);
 BENCHMARK_REGISTER_F(TracezAggregator, BM_ManyBucketsManyNames)->Arg(10)->Arg(1000);
-BENCHMARK_REGISTER_F(TracezAggregatorFetch, BM_ManyBucketsManyNames)->Arg(10)->Arg(1000);
+BENCHMARK_REGISTER_F(TracezAggregator, BM_ManyBucketsManyNamesFetch)->Arg(10)->Arg(1000);
 
 } // namespace zpages
 } // namespace ext
diff --git a/ext/zpages/test/tracez_processor_benchmark.cc b/ext/zpages/test/tracez_processor_benchmark.cc
index 78a4a3923d..7724b1d8e6 100644
--- a/ext/zpages/test/tracez_processor_benchmark.cc
+++ b/ext/zpages/test/tracez_processor_benchmark.cc
@@ -52,9 +52,9 @@ void GetManySnapshots(std::shared_ptr<TracezSpanProcessor> &processor, int num_s
 class TracezProcessorPeer
 {
 public:
-  TracezProcessorPeer(std::shared_ptr<TracezSpanProcessor> &processor_in)
+  TracezProcessorPeer(std::shared_ptr<TracezSpanProcessor> &processor)
   {
-    processor  = processor_in;
+    processor_  = processor;
   }
 
   /*
@@ -63,7 +63,7 @@ class TracezProcessorPeer
   void StartAllSpans(std::vector<std::unique_ptr<ThreadsafeSpanData>> &spans)
   {
     for(auto &span : spans)
-      processor->OnStart(*(span.get()));
+      processor_->OnStart(*(span.get()));
   }
 
   /*
@@ -74,21 +74,21 @@ class TracezProcessorPeer
   void EndAllSpans(std::vector<std::unique_ptr<ThreadsafeSpanData>> &spans)
   {
     for(auto &span : spans)
-      processor->OnEnd(std::move(span));
+      processor_->OnEnd(std::move(span));
     spans.clear();
   }
 
   /*
    * Clears running span pointers in processor memory, which is used in between
-   * iterations so that spans_.running doesn't hold nullptr
+   * iterations so that processor->spans_.running doesn't hold nullptr
    */
   void ClearRunning()
   {
-    processor->spans_.running.clear();
+    processor_->spans_.running.clear();
   }
 
 private:
-  std::shared_ptr<TracezSpanProcessor> processor;
+  std::shared_ptr<TracezSpanProcessor> processor_;
 };
 
 ////////////////////////  FIXTURE FOR SHARED SETUP CODE ///////////////////
@@ -98,12 +98,12 @@ class TracezProcessor : public benchmark::Fixture
 protected:
   void SetUp(const ::benchmark::State& state)
   {
-    processor  = std::shared_ptr<TracezSpanProcessor>(new TracezSpanProcessor());
-    processor_peer = std::unique_ptr<TracezProcessorPeer>(new TracezProcessorPeer(processor));
+    processor_  = std::shared_ptr<TracezSpanProcessor>(new TracezSpanProcessor());
+    processor_peer_ = std::unique_ptr<TracezProcessorPeer>(new TracezProcessorPeer(processor_));
   }
-  std::vector<std::unique_ptr<ThreadsafeSpanData>> spans;
-  std::unique_ptr<TracezProcessorPeer> processor_peer;
-  std::shared_ptr<TracezSpanProcessor> processor;
+  std::vector<std::unique_ptr<ThreadsafeSpanData>> spans_;
+  std::unique_ptr<TracezProcessorPeer> processor_peer_;
+  std::shared_ptr<TracezSpanProcessor> processor_;
 
 };
 
@@ -120,14 +120,14 @@ BENCHMARK_DEFINE_F(TracezProcessor, BM_MakeRunning)(benchmark::State &state)
   {
     state.PauseTiming();
     // Clear span vector if EndAllSpans for spans isn't called
-    spans.clear();
+    spans_.clear();
     // Clear processor memory, ensure no nullptrs are in running span set from
     // previous iterations from either spans or spans2 if EndAllSpans isn't called
-    processor_peer->ClearRunning();
-    CreateRecordables(spans, num_spans);
+    processor_peer_->ClearRunning();
+    CreateRecordables(spans_, num_spans);
     state.ResumeTiming();
 
-    processor_peer->StartAllSpans(spans);
+    processor_peer_->StartAllSpans(spans_);
   }
 }
 
@@ -141,11 +141,11 @@ BENCHMARK_DEFINE_F(TracezProcessor, BM_MakeComplete)(benchmark::State &state)
   for (auto _ : state)
   {
     state.PauseTiming();
-    CreateRecordables(spans, num_spans);
-    processor_peer->StartAllSpans(spans);
+    CreateRecordables(spans_, num_spans);
+    processor_peer_->StartAllSpans(spans_);
     state.ResumeTiming();
 
-    processor_peer->EndAllSpans(spans);
+    processor_peer_->EndAllSpans(spans_);
   }
 }
 
@@ -157,7 +157,7 @@ BENCHMARK_DEFINE_F(TracezProcessor, BM_GetSpans)(benchmark::State &state)
 {
   const int num_spans = state.range(0);
   for (auto _ : state)
-    GetManySnapshots(processor, num_spans);
+    GetManySnapshots(processor_, num_spans);
 }
 
 /*
@@ -170,22 +170,22 @@ BENCHMARK_DEFINE_F(TracezProcessor, BM_MakeRunningMakeComplete)(benchmark::State
   for (auto _ : state)
   {
     state.PauseTiming();
-    processor_peer->ClearRunning();
+    processor_peer_->ClearRunning();
     std::vector<std::unique_ptr<ThreadsafeSpanData>> spans2;
-    CreateRecordables(spans, num_spans);
+    CreateRecordables(spans_, num_spans);
     CreateRecordables(spans2, num_spans);
-    processor_peer->StartAllSpans(spans);
+    processor_peer_->StartAllSpans(spans_);
     state.ResumeTiming();
 
-    std::thread start(&TracezProcessorPeer::StartAllSpans, processor_peer.get(), std::ref(spans2));
-    processor_peer->EndAllSpans(spans);
+    std::thread start(&TracezProcessorPeer::StartAllSpans, processor_peer_.get(), std::ref(spans2));
+    processor_peer_->EndAllSpans(spans_);
 
     start.join();
   }
 }
 
 /*
- * Make many empty spans while spapshots grabbed. This checks the scenario where the
+ * Make many empty spans while snapshots grabbed. This checks the scenario where the
  * processor holds many running spans and gets queried.
  */
 BENCHMARK_DEFINE_F(TracezProcessor, BM_MakeRunningGetSpans)(benchmark::State &state)
@@ -194,13 +194,13 @@ BENCHMARK_DEFINE_F(TracezProcessor, BM_MakeRunningGetSpans)(benchmark::State &st
   for (auto _ : state)
   {
     state.PauseTiming();
-    spans.clear();
-    processor_peer->ClearRunning();
-    CreateRecordables(spans, num_spans);
+    spans_.clear();
+    processor_peer_->ClearRunning();
+    CreateRecordables(spans_, num_spans);
     state.ResumeTiming();
 
-    std::thread start(&TracezProcessorPeer::StartAllSpans, processor_peer.get(), std::ref(spans));
-    GetManySnapshots(processor, num_spans);
+    std::thread start(&TracezProcessorPeer::StartAllSpans, processor_peer_.get(), std::ref(spans_));
+    GetManySnapshots(processor_, num_spans);
 
     start.join();
   }
@@ -217,12 +217,12 @@ BENCHMARK_DEFINE_F(TracezProcessor, BM_GetSpansMakeComplete)(benchmark::State &s
   for (auto _ : state)
   {
     state.PauseTiming();
-    CreateRecordables(spans, num_spans);
-    processor_peer->StartAllSpans(spans);
+    CreateRecordables(spans_, num_spans);
+    processor_peer_->StartAllSpans(spans_);
     state.ResumeTiming();
 
-    std::thread end(&TracezProcessorPeer::EndAllSpans, processor_peer.get(), std::ref(spans));
-    GetManySnapshots(processor, num_spans);
+    std::thread end(&TracezProcessorPeer::EndAllSpans, processor_peer_.get(), std::ref(spans_));
+    GetManySnapshots(processor_, num_spans);
 
     end.join();
   }
@@ -239,16 +239,16 @@ BENCHMARK_DEFINE_F(TracezProcessor, BM_MakeRunningGetSpansMakeComplete)(benchmar
   for (auto _ : state)
   {
     state.PauseTiming();
-    processor_peer->ClearRunning();
+    processor_peer_->ClearRunning();
     std::vector<std::unique_ptr<ThreadsafeSpanData>> spans2;
-    CreateRecordables(spans, num_spans);
+    CreateRecordables(spans_, num_spans);
     CreateRecordables(spans2, num_spans);
-    processor_peer->StartAllSpans(spans);
+    processor_peer_->StartAllSpans(spans_);
     state.ResumeTiming();
 
-    std::thread end(&TracezProcessorPeer::EndAllSpans, processor_peer.get(), std::ref(spans));
-    std::thread start(&TracezProcessorPeer::StartAllSpans, processor_peer.get(), std::ref(spans2));
-    GetManySnapshots(processor, num_spans);
+    std::thread end(&TracezProcessorPeer::EndAllSpans, processor_peer_.get(), std::ref(spans_));
+    std::thread start(&TracezProcessorPeer::StartAllSpans, processor_peer_.get(), std::ref(spans2));
+    GetManySnapshots(processor_, num_spans);
 
     start.join();
     end.join();

From b11854471a858c482f1bf5ac84554ec62e8fd4ff Mon Sep 17 00:00:00 2001
From: Janet Vu <vjanet@umich.edu>
Date: Thu, 27 Aug 2020 15:49:46 +0000
Subject: [PATCH 36/40] Readabiliy, int->unsigned, makecomplete processor
 benchmark

---
 .../test/tracez_data_aggregator_benchmark.cc  | 24 ++++++++++---------
 ext/zpages/test/tracez_processor_benchmark.cc | 19 ++++++++-------
 2 files changed, 24 insertions(+), 19 deletions(-)

diff --git a/ext/zpages/test/tracez_data_aggregator_benchmark.cc b/ext/zpages/test/tracez_data_aggregator_benchmark.cc
index 9709d8379e..17f00ca953 100644
--- a/ext/zpages/test/tracez_data_aggregator_benchmark.cc
+++ b/ext/zpages/test/tracez_data_aggregator_benchmark.cc
@@ -24,14 +24,14 @@ namespace zpages
  */
 void StartEndSpans(
     std::shared_ptr<opentelemetry::trace::Tracer> &tracer,
-    int num_spans,
+    unsigned int num_spans,
     bool is_unique = false)
 {
   opentelemetry::trace::StartSpanOptions start;
   opentelemetry::trace::EndSpanOptions end;
   start.start_steady_time = SteadyTimestamp(nanoseconds(0));
   end.end_steady_time = SteadyTimestamp(nanoseconds(1));
-  for (; num_spans > 0; num_spans--)
+  for (unsigned int i = 0; i < num_spans; i++)
     tracer->StartSpan(is_unique ? std::to_string(num_spans) : "", start)->End(end);
 }
 
@@ -42,12 +42,12 @@ void StartEndSpans(
  */
 void StartEndSpansLatency(
     std::shared_ptr<opentelemetry::trace::Tracer> &tracer,
-    int num_spans,
+    unsigned int num_spans,
     bool is_unique = false)
 {
   opentelemetry::trace::StartSpanOptions start;
   start.start_steady_time = SteadyTimestamp(nanoseconds(0));
-  for (; num_spans > 0; num_spans--)
+  for (unsigned int i = 0; i < num_spans; i++)
   {
     // Latency bucket depends on the index
     nanoseconds latency_band = kLatencyBoundaries[num_spans % kLatencyBoundaries.size()];
@@ -65,10 +65,10 @@ void StartEndSpansLatency(
  */
 void StartEndSpansError(
     std::shared_ptr<opentelemetry::trace::Tracer> &tracer,
-    int num_spans,
+    unsigned int num_spans,
     bool is_unique = false)
 {
-  for (; num_spans > 0; num_spans--)
+  for (unsigned int i = 0; i < num_spans; i++)
     tracer->StartSpan(is_unique ? std::to_string(num_spans) : "")
         ->SetStatus(opentelemetry::trace::CanonicalCode::CANCELLED, "");
 }
@@ -81,24 +81,27 @@ void StartEndSpansError(
 void StartSpans(
     std::vector<opentelemetry::nostd::shared_ptr<opentelemetry::trace::Span>> &spans,
     std::shared_ptr<opentelemetry::trace::Tracer> &tracer,
-    int num_spans,
+    unsigned int num_spans,
     bool is_unique = false)
 {
-  for (; num_spans > 0; num_spans--)
+  for (unsigned int i = 0; i < num_spans; i++)
     spans.push_back(tracer->StartSpan(is_unique ? std::to_string(num_spans) : ""));
 }
 
 /*
  * Helper function that creates about num_spans spans, evenly split between running, latency (which
  * is further split by bucket), and error. If is_unique is true, then all spans will have
- * different names. The running spans vector is returned so that they remain running.
+ * different names. The running spans vector is returned so that they remain running. A vector is
+ * returned holding the running spans so they can be held in the caller function and remain running.
  */
 std::vector<opentelemetry::nostd::shared_ptr<opentelemetry::trace::Span>> MakeManySpans(
     std::shared_ptr<opentelemetry::trace::Tracer> &tracer,
     int num_spans,
     bool is_unique = false)
 {
-  // Running spans must be stored in a vector in order to stay running
+  // Running spans must be stored in a vector in order to stay running, since only OnStart is called
+  // for those spans. This vector is only accessed by the run thread, as the other functions' spans
+  // automatically get moved to the processor memory when calling both OnStart and OnEnd
   std::vector<opentelemetry::nostd::shared_ptr<opentelemetry::trace::Span>> running_spans;
   // Use threads to speed up the work
   std::thread run(StartSpans, std::ref(running_spans), std::ref(tracer), num_spans / 3, is_unique);
@@ -109,7 +112,6 @@ std::vector<opentelemetry::nostd::shared_ptr<opentelemetry::trace::Span>> MakeMa
   return running_spans;
 }
 
-
 /////////////////////////// AGGREGATOR PEER //////////////////////////////
 
 /*
diff --git a/ext/zpages/test/tracez_processor_benchmark.cc b/ext/zpages/test/tracez_processor_benchmark.cc
index 7724b1d8e6..a8dfc6c82a 100644
--- a/ext/zpages/test/tracez_processor_benchmark.cc
+++ b/ext/zpages/test/tracez_processor_benchmark.cc
@@ -21,9 +21,9 @@ namespace zpages
  */
 void CreateRecordables(
     std::vector<std::unique_ptr<ThreadsafeSpanData>> &spans,
-    int num_spans)
+    unsigned int num_spans)
 {
-  for (; num_spans > 0; num_spans--)
+  for (unsigned int i = 0; i < num_spans; i++)
     spans.push_back(std::unique_ptr<ThreadsafeSpanData>(new ThreadsafeSpanData()));
 }
 
@@ -34,9 +34,9 @@ void CreateRecordables(
  * simulates an aggregator querying the processor many times, but doesn't
  * aggregate or store spans.
  */
-void GetManySnapshots(std::shared_ptr<TracezSpanProcessor> &processor, int num_snapshots)
+void GetManySnapshots(std::shared_ptr<TracezSpanProcessor> &processor, unsigned int num_snapshots)
 {
-  for (; num_snapshots > 0; num_snapshots--)
+  for (unsigned int i = 0; i < num_snapshots; i++)
     processor->GetSpanSnapshot();
 }
 
@@ -119,10 +119,11 @@ BENCHMARK_DEFINE_F(TracezProcessor, BM_MakeRunning)(benchmark::State &state)
   for (auto _ : state)
   {
     state.PauseTiming();
-    // Clear span vector if EndAllSpans for spans isn't called
+    // Clear span vector if EndAllSpans for spans isn't called to avoid double
+    // starting spans
     spans_.clear();
-    // Clear processor memory, ensure no nullptrs are in running span set from
-    // previous iterations from either spans or spans2 if EndAllSpans isn't called
+    // Clear processor memory, which ensures no nullptrs are in running spans are
+    // set from previous iterations when EndAllSpans isn't called on span/spans2
     processor_peer_->ClearRunning();
     CreateRecordables(spans_, num_spans);
     state.ResumeTiming();
@@ -259,8 +260,10 @@ BENCHMARK_DEFINE_F(TracezProcessor, BM_MakeRunningGetSpansMakeComplete)(benchmar
 
 // Arg is the number of spans created for each iteration
 BENCHMARK_REGISTER_F(TracezProcessor, BM_MakeRunning)->Arg(10)->Arg(1000);
+BENCHMARK_REGISTER_F(TracezProcessor, BM_MakeComplete)->Arg(10)->Arg(1000);
 BENCHMARK_REGISTER_F(TracezProcessor, BM_GetSpans)->Arg(10)->Arg(1000);
-// These use multiple threads, so that CPU usage needs to be measured as well
+
+// These use multiple threads, so that CPU usage and thread works needs to be measured
 BENCHMARK_REGISTER_F(TracezProcessor, BM_MakeRunningMakeComplete)->Arg(10)->Arg(1000)
 	->MeasureProcessCPUTime()->UseRealTime();
 BENCHMARK_REGISTER_F(TracezProcessor, BM_MakeRunningGetSpans)->Arg(10)->Arg(1000)

From 61ea8bab8798024b060a9e664c22418d6325a4ad Mon Sep 17 00:00:00 2001
From: Janet Vu <vjanet@umich.edu>
Date: Thu, 27 Aug 2020 16:03:13 +0000
Subject: [PATCH 37/40] const int->const unsigned, var name clarifcation

---
 ext/zpages/test/tracez_processor_benchmark.cc | 30 +++++++++----------
 1 file changed, 15 insertions(+), 15 deletions(-)

diff --git a/ext/zpages/test/tracez_processor_benchmark.cc b/ext/zpages/test/tracez_processor_benchmark.cc
index a8dfc6c82a..88f981fc77 100644
--- a/ext/zpages/test/tracez_processor_benchmark.cc
+++ b/ext/zpages/test/tracez_processor_benchmark.cc
@@ -115,7 +115,7 @@ class TracezProcessor : public benchmark::Fixture
  */
 BENCHMARK_DEFINE_F(TracezProcessor, BM_MakeRunning)(benchmark::State &state)
 {
-  const int num_spans = state.range(0);
+  const unsigned int num_spans = state.range(0);
   for (auto _ : state)
   {
     state.PauseTiming();
@@ -138,7 +138,7 @@ BENCHMARK_DEFINE_F(TracezProcessor, BM_MakeRunning)(benchmark::State &state)
  */
 BENCHMARK_DEFINE_F(TracezProcessor, BM_MakeComplete)(benchmark::State &state)
 {
-  const int num_spans = state.range(0);
+  const unsigned int num_spans = state.range(0);
   for (auto _ : state)
   {
     state.PauseTiming();
@@ -156,9 +156,9 @@ BENCHMARK_DEFINE_F(TracezProcessor, BM_MakeComplete)(benchmark::State &state)
  */
 BENCHMARK_DEFINE_F(TracezProcessor, BM_GetSpans)(benchmark::State &state)
 {
-  const int num_spans = state.range(0);
+  const unsigned int num_snapshots = state.range(0);
   for (auto _ : state)
-    GetManySnapshots(processor_, num_spans);
+    GetManySnapshots(processor_, num_snapshots);
 }
 
 /*
@@ -167,7 +167,7 @@ BENCHMARK_DEFINE_F(TracezProcessor, BM_GetSpans)(benchmark::State &state)
  */
 BENCHMARK_DEFINE_F(TracezProcessor, BM_MakeRunningMakeComplete)(benchmark::State &state)
 {
-  const int num_spans = state.range(0);
+  const unsigned int num_spans = state.range(0);
   for (auto _ : state)
   {
     state.PauseTiming();
@@ -191,17 +191,17 @@ BENCHMARK_DEFINE_F(TracezProcessor, BM_MakeRunningMakeComplete)(benchmark::State
  */
 BENCHMARK_DEFINE_F(TracezProcessor, BM_MakeRunningGetSpans)(benchmark::State &state)
 {
-  const int num_spans = state.range(0);
+  const unsigned int num_spans_snaps = state.range(0); // number of spans and snapshots
   for (auto _ : state)
   {
     state.PauseTiming();
     spans_.clear();
     processor_peer_->ClearRunning();
-    CreateRecordables(spans_, num_spans);
+    CreateRecordables(spans_, num_spans_snaps);
     state.ResumeTiming();
 
     std::thread start(&TracezProcessorPeer::StartAllSpans, processor_peer_.get(), std::ref(spans_));
-    GetManySnapshots(processor_, num_spans);
+    GetManySnapshots(processor_, num_spans_snaps);
 
     start.join();
   }
@@ -214,16 +214,16 @@ BENCHMARK_DEFINE_F(TracezProcessor, BM_MakeRunningGetSpans)(benchmark::State &st
  */
 BENCHMARK_DEFINE_F(TracezProcessor, BM_GetSpansMakeComplete)(benchmark::State &state)
 {
-  const int num_spans = state.range(0);
+  const unsigned int num_spans_snaps = state.range(0);
   for (auto _ : state)
   {
     state.PauseTiming();
-    CreateRecordables(spans_, num_spans);
+    CreateRecordables(spans_, num_spans_snaps);
     processor_peer_->StartAllSpans(spans_);
     state.ResumeTiming();
 
     std::thread end(&TracezProcessorPeer::EndAllSpans, processor_peer_.get(), std::ref(spans_));
-    GetManySnapshots(processor_, num_spans);
+    GetManySnapshots(processor_, num_spans_snaps);
 
     end.join();
   }
@@ -236,20 +236,20 @@ BENCHMARK_DEFINE_F(TracezProcessor, BM_GetSpansMakeComplete)(benchmark::State &s
  */
 BENCHMARK_DEFINE_F(TracezProcessor, BM_MakeRunningGetSpansMakeComplete)(benchmark::State &state)
 {
-  const int num_spans = state.range(0);
+  const unsigned int num_spans_snaps = state.range(0);
   for (auto _ : state)
   {
     state.PauseTiming();
     processor_peer_->ClearRunning();
     std::vector<std::unique_ptr<ThreadsafeSpanData>> spans2;
-    CreateRecordables(spans_, num_spans);
-    CreateRecordables(spans2, num_spans);
+    CreateRecordables(spans_, num_spans_snaps);
+    CreateRecordables(spans2, num_spans_snaps);
     processor_peer_->StartAllSpans(spans_);
     state.ResumeTiming();
 
     std::thread end(&TracezProcessorPeer::EndAllSpans, processor_peer_.get(), std::ref(spans_));
     std::thread start(&TracezProcessorPeer::StartAllSpans, processor_peer_.get(), std::ref(spans2));
-    GetManySnapshots(processor_, num_spans);
+    GetManySnapshots(processor_, num_spans_snaps);
 
     start.join();
     end.join();

From c3c766103bb26707f080b71742461aa08333f7fe Mon Sep 17 00:00:00 2001
From: Janet Vu <vjanet@umich.edu>
Date: Thu, 27 Aug 2020 18:28:46 +0000
Subject: [PATCH 38/40] Add more details, deduplicate more code

---
 .../test/tracez_data_aggregator_benchmark.cc  | 181 ++++++------------
 1 file changed, 62 insertions(+), 119 deletions(-)

diff --git a/ext/zpages/test/tracez_data_aggregator_benchmark.cc b/ext/zpages/test/tracez_data_aggregator_benchmark.cc
index 17f00ca953..6715b1f4d1 100644
--- a/ext/zpages/test/tracez_data_aggregator_benchmark.cc
+++ b/ext/zpages/test/tracez_data_aggregator_benchmark.cc
@@ -96,7 +96,7 @@ void StartSpans(
  */
 std::vector<opentelemetry::nostd::shared_ptr<opentelemetry::trace::Span>> MakeManySpans(
     std::shared_ptr<opentelemetry::trace::Tracer> &tracer,
-    int num_spans,
+    unsigned int num_spans,
     bool is_unique = false)
 {
   // Running spans must be stored in a vector in order to stay running, since only OnStart is called
@@ -214,150 +214,93 @@ class TracezAggregator : public benchmark::Fixture
   std::shared_ptr<opentelemetry::trace::Tracer> tracer_;
 };
 
-////////////////////////// BENCHMARK HELPERS ///////////////////////////////
+//////////////////////////// BENCHMARK DEFINITIONS /////////////////////////////////
 
 /*
- * Aggregator handling many spans with the same name, who end instantly. This
- * checks the scenario where there's only one Tracez name and minimal sorting
- * of spans into different latency bands is required, as all spans should be
- * sorted in the same bucket under the same span name.
+ * Aggregator handling many spans where minimal sorting of spans into different
+ * latency bands is required, as all spans should be sorted in the same bucket.
+ * It also means there's minimal memory usage for the Tracez data structure holding
+ * sampled spans.
  */
-void SingleBucketSingleName(
-    benchmark::State &state,
-    std::shared_ptr<opentelemetry::trace::Tracer> &tracer,
-    std::unique_ptr<TracezDataAggregatorPeer> const &aggregator_peer)
+BENCHMARK_DEFINE_F(TracezAggregator, BM_SingleBucket)(benchmark::State &state)
 {
+  const unsigned int num_spans = state.range(0);
+  const bool is_unique = state.range(1);
+  const bool run_periodic_query = state.range(2);
+
+  if (run_periodic_query)
+    aggregator_peer_->StartPeriodicQueryThread();
+
   for (auto _ : state)
   {
     // Do not time span creation, as we're only benchmarking aggregation work
     state.PauseTiming();
-    StartEndSpans(tracer, state.range(0));
+    StartEndSpans(tracer_, num_spans, is_unique);
     state.ResumeTiming();
-    aggregator_peer->Aggregate();
+    aggregator_peer_->Aggregate();
   }
 }
 
 /*
- * Aggregator handling many spans with unique names, who end instantly. This
- * checks the scenario where there's many Tracez groups but minimal sorting
- * of spans into different latency bands is required. Spans are sorted in
- * different groups but always in the same bucket.
+ * Aggregator handling many spans who may fall under error, running, and any
+ * latency group. This requires the aggregator to sorting of spans into their
+ * respective buckets (running, error, latency [including within latency bands]),
+ * which also utilizes more memory.
  */
-void SingleBucketManyNames(
-    benchmark::State &state,
-    std::shared_ptr<opentelemetry::trace::Tracer> &tracer,
-    std::unique_ptr<TracezDataAggregatorPeer> const &aggregator_peer)
+BENCHMARK_DEFINE_F(TracezAggregator, BM_ManyBuckets)(benchmark::State &state)
 {
-  for (auto _ : state)
-  {
-    state.PauseTiming();
-    StartEndSpans(tracer, state.range(0), true);
-    state.ResumeTiming();
-    aggregator_peer->Aggregate();
-  }
-}
+  const unsigned int num_spans = state.range(0);
+  const bool is_unique = state.range(1);
+  const bool run_periodic_query = state.range(2);
 
-/*
- * Aggregator handling many spans with the same name, who may fall under error,
- * running, and any latency group. This checks the scenario where there's only
- * one Tracez groups but there also needs to be sorting of spans into their
- * respective buckets (running, error, latency [including within latency bands]).
- * Spans are in the same group but sorted to different buckets.
- */
-void ManyBucketsSingleName(
-    benchmark::State &state,
-    std::shared_ptr<opentelemetry::trace::Tracer> &tracer,
-    std::unique_ptr<TracezDataAggregatorPeer> const &aggregator_peer)
-{
-  for (auto _ : state)
-  {
-    state.PauseTiming();
-    auto running_spans = MakeManySpans(tracer, state.range(0));
-    state.ResumeTiming();
-    aggregator_peer->Aggregate();
-  }
-}
+  if (run_periodic_query)
+    aggregator_peer_->StartPeriodicQueryThread();
 
-/*
- * Aggregator handling many spans with unique names, who may fall under error,
- * running, and any latency group. This checks the scenario where there's many
- * Tracez groups and there also needs to be sorting of spans into their
- * respective buckets (running, error, latency [including within latency bands]).
- * Spans are in the same group but sorted to different buckets.
- */
-void ManyBucketsManyNames(
-    benchmark::State &state,
-    std::shared_ptr<opentelemetry::trace::Tracer> &tracer,
-    std::unique_ptr<TracezDataAggregatorPeer> const &aggregator_peer)
-{
   for (auto _ : state)
   {
     state.PauseTiming();
-    auto running_spans = MakeManySpans(tracer, state.range(0));
+    std::vector<opentelemetry::nostd::shared_ptr<
+    	opentelemetry::trace::Span>> running_spans = MakeManySpans(tracer_, num_spans, is_unique);
     state.ResumeTiming();
-    aggregator_peer->Aggregate();
+    aggregator_peer_->Aggregate();
   }
 }
 
-//////////////////////// USER NEVER VISITS WEBPAGE //////////////////////////////
-
-BENCHMARK_DEFINE_F(TracezAggregator, BM_SingleBucketSingleName)(benchmark::State &state)
-{
-  SingleBucketSingleName(state, tracer_, aggregator_peer_);
-}
-
-BENCHMARK_DEFINE_F(TracezAggregator, BM_SingleBucketManyNames)(benchmark::State &state)
-{
-  SingleBucketManyNames(state, tracer_, aggregator_peer_);
-}
-
-BENCHMARK_DEFINE_F(TracezAggregator, BM_ManyBucketsSingleName)(benchmark::State &state)
-{
-  ManyBucketsSingleName(state, tracer_, aggregator_peer_);
-}
-
-BENCHMARK_DEFINE_F(TracezAggregator, BM_ManyBucketsManyNames)(benchmark::State &state)
-{
-  ManyBucketsManyNames(state, tracer_, aggregator_peer_);
-}
-
-///////////// SAME BENCHMARKS, BUT AGGREGATIONS REQUESTED PERIODICALLY /////////////////
-
-BENCHMARK_DEFINE_F(TracezAggregator, BM_SingleBucketSingleNameFetch)(benchmark::State &state)
-{
-  aggregator_peer_->StartPeriodicQueryThread();
-  SingleBucketSingleName(state, tracer_, aggregator_peer_);
-}
-
-BENCHMARK_DEFINE_F(TracezAggregator, BM_SingleBucketManyNamesFetch)(benchmark::State &state)
-{
-  aggregator_peer_->StartPeriodicQueryThread();
-  SingleBucketManyNames(state, tracer_, aggregator_peer_);
-}
-
-BENCHMARK_DEFINE_F(TracezAggregator, BM_ManyBucketsSingleNameFetch)(benchmark::State &state)
-{
-  aggregator_peer_->StartPeriodicQueryThread();
-  ManyBucketsSingleName(state, tracer_, aggregator_peer_);
-}
-
-BENCHMARK_DEFINE_F(TracezAggregator, BM_ManyBucketsManyNamesFetch)(benchmark::State &state)
-{
-  aggregator_peer_->StartPeriodicQueryThread();
-  ManyBucketsManyNames(state, tracer_, aggregator_peer_);
-}
-
 //////////////////////////// RUN BENCHMARKS ///////////////////////////////
 
-// Arg is the number of spans created for each iteration
-BENCHMARK_REGISTER_F(TracezAggregator, BM_SingleBucketSingleName)->Arg(10)->Arg(1000);
-BENCHMARK_REGISTER_F(TracezAggregator, BM_SingleBucketSingleNameFetch)->Arg(10)->Arg(1000);
-BENCHMARK_REGISTER_F(TracezAggregator, BM_SingleBucketManyNames)->Arg(10)->Arg(1000);
-BENCHMARK_REGISTER_F(TracezAggregator, BM_SingleBucketManyNamesFetch)->Arg(10)->Arg(1000);
-BENCHMARK_REGISTER_F(TracezAggregator, BM_ManyBucketsSingleName)->Arg(10)->Arg(1000);
-BENCHMARK_REGISTER_F(TracezAggregator, BM_ManyBucketsSingleNameFetch)->Arg(10)->Arg(1000);
-BENCHMARK_REGISTER_F(TracezAggregator, BM_ManyBucketsManyNames)->Arg(10)->Arg(1000);
-BENCHMARK_REGISTER_F(TracezAggregator, BM_ManyBucketsManyNamesFetch)->Arg(10)->Arg(1000);
+/* Args: spans, is_unique, run_periodic_query
+ *
+ * @num_spans int number of spans created for each iteration
+ * @is_unique bool whether spans are unique or not
+ * @run_periodic_query bool whether to run a background periodic thread of not
+ */
+
+BENCHMARK_REGISTER_F(TracezAggregator, BM_SingleBucket)
+  /*
+   * Same name spans, with different number of spans and periodic query status
+   *
+   * Spans all have the same name, which means there's work to clear stored spans
+   * from memory fairly often so that the number of sampled spans won't go over
+   * the max and memory use is minimal, which are performance factors
+   */
+  ->Args({10, false, false})->Args({10, false, true})
+  ->Args({1000, false, false})->Args({1000, false, true})
+  /*
+   * Many name spans, with different number of spans and periodic query status
+   *
+   * Spans have num_spans unique names, so many more spans are kept stored in memory
+   * and not cleared as often, which affects performance
+   */
+  ->Args({10, true, false})->Args({10, true, true})
+  ->Args({1000, true, false})->Args({1000, true, true});
+
+// Do same permutation of number of spans, span uniqueness, and periodic thread
+// running for many buckets too
+BENCHMARK_REGISTER_F(TracezAggregator, BM_ManyBuckets)
+  ->Args({10, false, false})->Args({10, false, true})
+  ->Args({1000, false, false})->Args({1000, false, true})
+  ->Args({10, true, false})->Args({10, true, true})
+  ->Args({1000, true, false})->Args({1000, true, true});
 
 } // namespace zpages
 } // namespace ext

From 033a980c5385d650090c47a7f03de5c0f6cb9440 Mon Sep 17 00:00:00 2001
From: Janet Vu <vjanet@umich.edu>
Date: Sat, 29 Aug 2020 01:04:47 +0000
Subject: [PATCH 39/40] Ensure all latency bands filled, run formatter

---
 ext/zpages/test/BUILD                         |   4 +-
 .../test/tracez_data_aggregator_benchmark.cc  | 136 ++++++++++--------
 ext/zpages/test/tracez_processor_benchmark.cc |  61 ++++----
 3 files changed, 108 insertions(+), 93 deletions(-)

diff --git a/ext/zpages/test/BUILD b/ext/zpages/test/BUILD
index 646cedeeab..b642625918 100644
--- a/ext/zpages/test/BUILD
+++ b/ext/zpages/test/BUILD
@@ -42,9 +42,9 @@ otel_cc_benchmark(
         "tracez_data_aggregator_benchmark.cc",
     ],
     deps = [
+        "//api",
         "//ext/zpages",
         "//sdk/src/trace",
-        "//api",
     ],
 )
 
@@ -54,8 +54,8 @@ otel_cc_benchmark(
         "tracez_processor_benchmark.cc",
     ],
     deps = [
+        "//api",
         "//ext/zpages",
         "//sdk/src/trace",
-        "//api",
     ],
 )
diff --git a/ext/zpages/test/tracez_data_aggregator_benchmark.cc b/ext/zpages/test/tracez_data_aggregator_benchmark.cc
index 6715b1f4d1..ad5925fec4 100644
--- a/ext/zpages/test/tracez_data_aggregator_benchmark.cc
+++ b/ext/zpages/test/tracez_data_aggregator_benchmark.cc
@@ -22,15 +22,14 @@ namespace zpages
  * Helper function that creates and ends num_spans spans instantly. If is_unique is
  * true, then all spans will have different names.
  */
-void StartEndSpans(
-    std::shared_ptr<opentelemetry::trace::Tracer> &tracer,
-    unsigned int num_spans,
-    bool is_unique = false)
+void StartEndSpans(std::shared_ptr<opentelemetry::trace::Tracer> &tracer,
+                   unsigned int num_spans,
+                   bool is_unique)
 {
   opentelemetry::trace::StartSpanOptions start;
   opentelemetry::trace::EndSpanOptions end;
   start.start_steady_time = SteadyTimestamp(nanoseconds(0));
-  end.end_steady_time = SteadyTimestamp(nanoseconds(1));
+  end.end_steady_time     = SteadyTimestamp(nanoseconds(1));
   for (unsigned int i = 0; i < num_spans; i++)
     tracer->StartSpan(is_unique ? std::to_string(num_spans) : "", start)->End(end);
 }
@@ -40,17 +39,19 @@ void StartEndSpans(
  * spreading spans across all latency bands. If is_unique is true, then all spans
  * will have different names.
  */
-void StartEndSpansLatency(
-    std::shared_ptr<opentelemetry::trace::Tracer> &tracer,
-    unsigned int num_spans,
-    bool is_unique = false)
+void StartEndSpansLatency(std::shared_ptr<opentelemetry::trace::Tracer> &tracer,
+                          unsigned int num_spans,
+                          bool is_unique,
+                          unsigned int latency_span_offset)
 {
   opentelemetry::trace::StartSpanOptions start;
   start.start_steady_time = SteadyTimestamp(nanoseconds(0));
   for (unsigned int i = 0; i < num_spans; i++)
   {
-    // Latency bucket depends on the index
-    nanoseconds latency_band = kLatencyBoundaries[num_spans % kLatencyBoundaries.size()];
+    // Latency bucket depends on the index. Pass-by-ref offset means each span name will have all
+    // buckets filled
+    nanoseconds latency_band =
+        kLatencyBoundaries[num_spans + latency_span_offset % kLatencyBoundaries.size()];
     opentelemetry::trace::EndSpanOptions end;
     end.end_steady_time = SteadyTimestamp(latency_band);
 
@@ -63,13 +64,14 @@ void StartEndSpansLatency(
  * simulating error codes. If is_unique is true, then all spans will have
  * different names.
  */
-void StartEndSpansError(
-    std::shared_ptr<opentelemetry::trace::Tracer> &tracer,
-    unsigned int num_spans,
-    bool is_unique = false)
+void StartEndSpansError(std::shared_ptr<opentelemetry::trace::Tracer> &tracer,
+                        unsigned int num_spans,
+                        bool is_unique)
 {
   for (unsigned int i = 0; i < num_spans; i++)
-    tracer->StartSpan(is_unique ? std::to_string(num_spans) : "")
+    tracer
+        ->StartSpan(is_unique ? std::to_string(num_spans) : "")
+        // Random error status
         ->SetStatus(opentelemetry::trace::CanonicalCode::CANCELLED, "");
 }
 
@@ -78,11 +80,10 @@ void StartEndSpansError(
  * spans are referenced in the passed in vector. If is_unique is true, then all spans
  * will have different names.
  */
-void StartSpans(
-    std::vector<opentelemetry::nostd::shared_ptr<opentelemetry::trace::Span>> &spans,
-    std::shared_ptr<opentelemetry::trace::Tracer> &tracer,
-    unsigned int num_spans,
-    bool is_unique = false)
+void StartSpans(std::vector<opentelemetry::nostd::shared_ptr<opentelemetry::trace::Span>> &spans,
+                std::shared_ptr<opentelemetry::trace::Tracer> &tracer,
+                unsigned int num_spans,
+                bool is_unique)
 {
   for (unsigned int i = 0; i < num_spans; i++)
     spans.push_back(tracer->StartSpan(is_unique ? std::to_string(num_spans) : ""));
@@ -97,18 +98,21 @@ void StartSpans(
 std::vector<opentelemetry::nostd::shared_ptr<opentelemetry::trace::Span>> MakeManySpans(
     std::shared_ptr<opentelemetry::trace::Tracer> &tracer,
     unsigned int num_spans,
-    bool is_unique = false)
+    bool is_unique,
+    unsigned int &latency_span_offset)
 {
   // Running spans must be stored in a vector in order to stay running, since only OnStart is called
   // for those spans. This vector is only accessed by the run thread, as the other functions' spans
   // automatically get moved to the processor memory when calling both OnStart and OnEnd
   std::vector<opentelemetry::nostd::shared_ptr<opentelemetry::trace::Span>> running_spans;
   // Use threads to speed up the work
-  std::thread run(StartSpans, std::ref(running_spans), std::ref(tracer), num_spans / 3, is_unique);
-  std::thread err(StartEndSpansError, std::ref(tracer), num_spans / 3, is_unique);
-  StartEndSpansLatency(tracer, num_spans / 3, is_unique);
+  std::thread run(StartSpans, std::ref(running_spans), std::ref(tracer), num_spans / 6, is_unique);
+  std::thread err(StartEndSpansError, std::ref(tracer), num_spans / 6, is_unique);
+  StartEndSpansLatency(tracer, num_spans * 3 / 4, is_unique, latency_span_offset);
   run.join();
   err.join();
+  // Increment offset for later
+  latency_span_offset++;
   return running_spans;
 }
 
@@ -124,8 +128,7 @@ class TracezDataAggregatorPeer
   TracezDataAggregatorPeer(std::shared_ptr<TracezSpanProcessor> processor)
   {
     // Set up the aggregator
-    aggregator_ = std::unique_ptr<TracezDataAggregator>(
-        new TracezDataAggregator(processor));
+    aggregator_ = std::unique_ptr<TracezDataAggregator>(new TracezDataAggregator(processor));
 
     // Disable the aggregetor's periodic background thread aggregation work, which
     // it normally does during production. Disabling it allows us to isolate
@@ -189,7 +192,6 @@ class TracezDataAggregatorPeer
   std::mutex mtx_;
   std::atomic<bool> run_;
   std::condition_variable cont_;
-  
 };
 
 ////////////////////////  FIXTURE FOR SHARED SETUP CODE ///////////////////
@@ -201,12 +203,12 @@ class TracezDataAggregatorPeer
 class TracezAggregator : public benchmark::Fixture
 {
 protected:
-  void SetUp(const ::benchmark::State& state)
+  void SetUp(const ::benchmark::State &state)
   {
     std::shared_ptr<TracezSpanProcessor> processor(new TracezSpanProcessor());
     tracer_ = std::shared_ptr<opentelemetry::trace::Tracer>(new Tracer(processor));
-    aggregator_peer_ = std::unique_ptr<TracezDataAggregatorPeer>(
-        new TracezDataAggregatorPeer(processor));
+    aggregator_peer_ =
+        std::unique_ptr<TracezDataAggregatorPeer>(new TracezDataAggregatorPeer(processor));
   }
 
   std::unique_ptr<TracezDataAggregatorPeer> aggregator_peer_;
@@ -224,8 +226,8 @@ class TracezAggregator : public benchmark::Fixture
  */
 BENCHMARK_DEFINE_F(TracezAggregator, BM_SingleBucket)(benchmark::State &state)
 {
-  const unsigned int num_spans = state.range(0);
-  const bool is_unique = state.range(1);
+  const unsigned int num_spans  = state.range(0);
+  const bool is_unique          = state.range(1);
   const bool run_periodic_query = state.range(2);
 
   if (run_periodic_query)
@@ -249,18 +251,21 @@ BENCHMARK_DEFINE_F(TracezAggregator, BM_SingleBucket)(benchmark::State &state)
  */
 BENCHMARK_DEFINE_F(TracezAggregator, BM_ManyBuckets)(benchmark::State &state)
 {
-  const unsigned int num_spans = state.range(0);
-  const bool is_unique = state.range(1);
+  const unsigned int num_spans  = state.range(0);
+  const bool is_unique          = state.range(1);
   const bool run_periodic_query = state.range(2);
 
+  // Ensure spans get added to each latency bucket
+  unsigned int latency_index_offset = 0;
+
   if (run_periodic_query)
     aggregator_peer_->StartPeriodicQueryThread();
 
   for (auto _ : state)
   {
     state.PauseTiming();
-    std::vector<opentelemetry::nostd::shared_ptr<
-    	opentelemetry::trace::Span>> running_spans = MakeManySpans(tracer_, num_spans, is_unique);
+    std::vector<opentelemetry::nostd::shared_ptr<opentelemetry::trace::Span>> running_spans =
+        MakeManySpans(tracer_, num_spans, is_unique, latency_index_offset);
     state.ResumeTiming();
     aggregator_peer_->Aggregate();
   }
@@ -276,35 +281,42 @@ BENCHMARK_DEFINE_F(TracezAggregator, BM_ManyBuckets)(benchmark::State &state)
  */
 
 BENCHMARK_REGISTER_F(TracezAggregator, BM_SingleBucket)
-  /*
-   * Same name spans, with different number of spans and periodic query status
-   *
-   * Spans all have the same name, which means there's work to clear stored spans
-   * from memory fairly often so that the number of sampled spans won't go over
-   * the max and memory use is minimal, which are performance factors
-   */
-  ->Args({10, false, false})->Args({10, false, true})
-  ->Args({1000, false, false})->Args({1000, false, true})
-  /*
-   * Many name spans, with different number of spans and periodic query status
-   *
-   * Spans have num_spans unique names, so many more spans are kept stored in memory
-   * and not cleared as often, which affects performance
-   */
-  ->Args({10, true, false})->Args({10, true, true})
-  ->Args({1000, true, false})->Args({1000, true, true});
+    /*
+     * Same name spans, with different number of spans and periodic query status
+     *
+     * Spans all have the same name, which means there's work to clear stored spans
+     * from memory fairly often so that the number of sampled spans won't go over
+     * the max and memory use is minimal, which are performance factors
+     */
+    ->Args({10, false, false})
+    ->Args({10, false, true})
+    ->Args({1000, false, false})
+    ->Args({1000, false, true})
+    /*
+     * Many name spans, with different number of spans and periodic query status
+     *
+     * Spans have num_spans unique names, so many more spans are kept stored in memory
+     * and not cleared as often, which affects performance
+     */
+    ->Args({10, true, false})
+    ->Args({10, true, true})
+    ->Args({1000, true, false})
+    ->Args({1000, true, true});
 
 // Do same permutation of number of spans, span uniqueness, and periodic thread
 // running for many buckets too
 BENCHMARK_REGISTER_F(TracezAggregator, BM_ManyBuckets)
-  ->Args({10, false, false})->Args({10, false, true})
-  ->Args({1000, false, false})->Args({1000, false, true})
-  ->Args({10, true, false})->Args({10, true, true})
-  ->Args({1000, true, false})->Args({1000, true, true});
-
-} // namespace zpages
-} // namespace ext
+    ->Args({10, false, false})
+    ->Args({10, false, true})
+    ->Args({1000, false, false})
+    ->Args({1000, false, true})
+    ->Args({10, true, false})
+    ->Args({10, true, true})
+    ->Args({1000, true, false})
+    ->Args({1000, true, true});
+
+}  // namespace zpages
+}  // namespace ext
 OPENTELEMETRY_END_NAMESPACE
 
 BENCHMARK_MAIN();
-
diff --git a/ext/zpages/test/tracez_processor_benchmark.cc b/ext/zpages/test/tracez_processor_benchmark.cc
index 88f981fc77..297c6fffa2 100644
--- a/ext/zpages/test/tracez_processor_benchmark.cc
+++ b/ext/zpages/test/tracez_processor_benchmark.cc
@@ -19,9 +19,8 @@ namespace zpages
  * Helper function that creates num_spans threadsafe span data, adding them to the spans
  * vector during setup to benchmark OnStart and OnEnd times for the processor.
  */
-void CreateRecordables(
-    std::vector<std::unique_ptr<ThreadsafeSpanData>> &spans,
-    unsigned int num_spans)
+void CreateRecordables(std::vector<std::unique_ptr<ThreadsafeSpanData>> &spans,
+                       unsigned int num_spans)
 {
   for (unsigned int i = 0; i < num_spans; i++)
     spans.push_back(std::unique_ptr<ThreadsafeSpanData>(new ThreadsafeSpanData()));
@@ -52,17 +51,14 @@ void GetManySnapshots(std::shared_ptr<TracezSpanProcessor> &processor, unsigned
 class TracezProcessorPeer
 {
 public:
-  TracezProcessorPeer(std::shared_ptr<TracezSpanProcessor> &processor)
-  {
-    processor_  = processor;
-  }
+  TracezProcessorPeer(std::shared_ptr<TracezSpanProcessor> &processor) { processor_ = processor; }
 
   /*
    * Calls TracezProcessor::OnStart on each span in spans.
    */
   void StartAllSpans(std::vector<std::unique_ptr<ThreadsafeSpanData>> &spans)
   {
-    for(auto &span : spans)
+    for (auto &span : spans)
       processor_->OnStart(*(span.get()));
   }
 
@@ -73,7 +69,7 @@ class TracezProcessorPeer
    */
   void EndAllSpans(std::vector<std::unique_ptr<ThreadsafeSpanData>> &spans)
   {
-    for(auto &span : spans)
+    for (auto &span : spans)
       processor_->OnEnd(std::move(span));
     spans.clear();
   }
@@ -82,10 +78,7 @@ class TracezProcessorPeer
    * Clears running span pointers in processor memory, which is used in between
    * iterations so that processor->spans_.running doesn't hold nullptr
    */
-  void ClearRunning()
-  {
-    processor_->spans_.running.clear();
-  }
+  void ClearRunning() { processor_->spans_.running.clear(); }
 
 private:
   std::shared_ptr<TracezSpanProcessor> processor_;
@@ -96,15 +89,14 @@ class TracezProcessorPeer
 class TracezProcessor : public benchmark::Fixture
 {
 protected:
-  void SetUp(const ::benchmark::State& state)
+  void SetUp(const ::benchmark::State &state)
   {
-    processor_  = std::shared_ptr<TracezSpanProcessor>(new TracezSpanProcessor());
+    processor_      = std::shared_ptr<TracezSpanProcessor>(new TracezSpanProcessor());
     processor_peer_ = std::unique_ptr<TracezProcessorPeer>(new TracezProcessorPeer(processor_));
   }
   std::vector<std::unique_ptr<ThreadsafeSpanData>> spans_;
   std::unique_ptr<TracezProcessorPeer> processor_peer_;
   std::shared_ptr<TracezSpanProcessor> processor_;
-
 };
 
 //////////////////////////// BENCHMARK DEFINITIONS /////////////////////////////////
@@ -191,7 +183,7 @@ BENCHMARK_DEFINE_F(TracezProcessor, BM_MakeRunningMakeComplete)(benchmark::State
  */
 BENCHMARK_DEFINE_F(TracezProcessor, BM_MakeRunningGetSpans)(benchmark::State &state)
 {
-  const unsigned int num_spans_snaps = state.range(0); // number of spans and snapshots
+  const unsigned int num_spans_snaps = state.range(0);  // number of spans and snapshots
   for (auto _ : state)
   {
     state.PauseTiming();
@@ -264,18 +256,29 @@ BENCHMARK_REGISTER_F(TracezProcessor, BM_MakeComplete)->Arg(10)->Arg(1000);
 BENCHMARK_REGISTER_F(TracezProcessor, BM_GetSpans)->Arg(10)->Arg(1000);
 
 // These use multiple threads, so that CPU usage and thread works needs to be measured
-BENCHMARK_REGISTER_F(TracezProcessor, BM_MakeRunningMakeComplete)->Arg(10)->Arg(1000)
-	->MeasureProcessCPUTime()->UseRealTime();
-BENCHMARK_REGISTER_F(TracezProcessor, BM_MakeRunningGetSpans)->Arg(10)->Arg(1000)
-	->MeasureProcessCPUTime()->UseRealTime();
-BENCHMARK_REGISTER_F(TracezProcessor, BM_GetSpansMakeComplete)->Arg(10)->Arg(1000)
-	->MeasureProcessCPUTime()->UseRealTime();
-BENCHMARK_REGISTER_F(TracezProcessor, BM_MakeRunningGetSpansMakeComplete)->Arg(10)->Arg(1000)
-	->MeasureProcessCPUTime()->UseRealTime();
-
-} // namespace zpages
-} // namespace ext
+BENCHMARK_REGISTER_F(TracezProcessor, BM_MakeRunningMakeComplete)
+    ->Arg(10)
+    ->Arg(1000)
+    ->MeasureProcessCPUTime()
+    ->UseRealTime();
+BENCHMARK_REGISTER_F(TracezProcessor, BM_MakeRunningGetSpans)
+    ->Arg(10)
+    ->Arg(1000)
+    ->MeasureProcessCPUTime()
+    ->UseRealTime();
+BENCHMARK_REGISTER_F(TracezProcessor, BM_GetSpansMakeComplete)
+    ->Arg(10)
+    ->Arg(1000)
+    ->MeasureProcessCPUTime()
+    ->UseRealTime();
+BENCHMARK_REGISTER_F(TracezProcessor, BM_MakeRunningGetSpansMakeComplete)
+    ->Arg(10)
+    ->Arg(1000)
+    ->MeasureProcessCPUTime()
+    ->UseRealTime();
+
+}  // namespace zpages
+}  // namespace ext
 OPENTELEMETRY_END_NAMESPACE
 
 BENCHMARK_MAIN();
-

From 2d10a238ac9f9244e4c96d12188f37ef87db9f04 Mon Sep 17 00:00:00 2001
From: Janet Vu <vjanet@umich.edu>
Date: Sat, 29 Aug 2020 01:11:54 +0000
Subject: [PATCH 40/40] Better wording and add medium article link

---
 ext/zpages/README.md | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/ext/zpages/README.md b/ext/zpages/README.md
index 455ae3a54d..db10fff5bb 100644
--- a/ext/zpages/README.md
+++ b/ext/zpages/README.md
@@ -1,6 +1,6 @@
 # zPages
 ## Overview
-zPages are a quick and light way to view tracing and metrics information on standard OpenTelemetry C++ instrumented applications. It requires no external dependencies or backend setup. See more information in the OTel zPages experimental [spec](https://github.com/open-telemetry/opentelemetry-specification/blob/5b86d4b6c42e6d1e47d9155ac1e2e27f0f0b7769/experimental/trace/zpages.md). OTel C++ currently only offers Tracez; future zPages to potentially add include TraceConfigz, RPCz, and Statsz. Events and links need to be added to Tracez.
+zPages are a quick and light way to view tracing and metrics information on standard OpenTelemetry C++ instrumented applications. It requires no external dependencies or backend setup. See more information in the OTel zPages experimental [spec](https://github.com/open-telemetry/opentelemetry-specification/blob/5b86d4b6c42e6d1e47d9155ac1e2e27f0f0b7769/experimental/trace/zpages.md). OTel C++ currently only offers Tracez, which are currently not showing events and links; future zPages to potentially add include TraceConfigz, RPCz, and Statsz. There's also a Medium article on zPages [here](https://medium.com/opentelemetry/zpages-in-opentelemetry-2b080a81eb47).
 
 # Usage
 > TODO: Add CMake instructions