From 27adfdee9a5724e3c8cb895da5347dc68296a486 Mon Sep 17 00:00:00 2001 From: Janet Vu Date: Wed, 8 Jul 2020 17:59:21 +0000 Subject: [PATCH 01/19] Clean add of tracez processor files --- ext/BUILD | 7 + ext/CMakeLists.txt | 5 + .../ext/zpages/tracez_processor.h | 104 ++++ ext/src/CMakeLists.txt | 1 + ext/src/zpages/BUILD | 27 + ext/src/zpages/CMakeLists.txt | 4 + ext/src/zpages/tracez_processor.cc | 34 ++ ext/test/CMakeLists.txt | 1 + ext/test/zpages/BUILD | 11 + ext/test/zpages/CMakeLists.txt | 16 + ext/test/zpages/tracez_processor_test.cc | 464 ++++++++++++++++++ 11 files changed, 674 insertions(+) create mode 100644 ext/BUILD create mode 100644 ext/CMakeLists.txt create mode 100644 ext/include/opentelemetry/ext/zpages/tracez_processor.h create mode 100644 ext/src/CMakeLists.txt create mode 100644 ext/src/zpages/BUILD create mode 100644 ext/src/zpages/CMakeLists.txt create mode 100644 ext/src/zpages/tracez_processor.cc create mode 100644 ext/test/CMakeLists.txt create mode 100644 ext/test/zpages/BUILD create mode 100644 ext/test/zpages/CMakeLists.txt create mode 100644 ext/test/zpages/tracez_processor_test.cc diff --git a/ext/BUILD b/ext/BUILD new file mode 100644 index 0000000000..cc62431b53 --- /dev/null +++ b/ext/BUILD @@ -0,0 +1,7 @@ +package(default_visibility = ["//visibility:public"]) + +cc_library( + name = "headers", + hdrs = glob(["include/**/*.h"]), + strip_include_prefix = "include", +) diff --git a/ext/CMakeLists.txt b/ext/CMakeLists.txt new file mode 100644 index 0000000000..75205ac71e --- /dev/null +++ b/ext/CMakeLists.txt @@ -0,0 +1,5 @@ +add_subdirectory(src) + +if(BUILD_TESTING) + add_subdirectory(test) +endif() diff --git a/ext/include/opentelemetry/ext/zpages/tracez_processor.h b/ext/include/opentelemetry/ext/zpages/tracez_processor.h new file mode 100644 index 0000000000..d1de94a52f --- /dev/null +++ b/ext/include/opentelemetry/ext/zpages/tracez_processor.h @@ -0,0 +1,104 @@ +#pragma once + +#include +#include +#include +#include +#include +#include +#include + +#include "opentelemetry/sdk/trace/recordable.h" +#include "opentelemetry/sdk/trace/span_data.h" +#include "opentelemetry/sdk/trace/exporter.h" +#include "opentelemetry/sdk/trace/processor.h" + +OPENTELEMETRY_BEGIN_NAMESPACE +namespace ext +{ +namespace zpages +{ +/** + * The span processor passes finished recordables to be used by the + * Data Aggregator to store spans before aggregating for TraceZ. + * + */ +class TracezSpanProcessor : public opentelemetry::sdk::trace::SpanProcessor { + public: + + struct CollectedSpans { + std::unordered_set running; + std::vector> completed; + }; + + /** + * Initialize a span processor. + */ + explicit TracezSpanProcessor() noexcept {} + + /** + * Create a span recordable + * @return a newly initialized recordable + */ + std::unique_ptr MakeRecordable() noexcept override + { + return std::unique_ptr(new opentelemetry::sdk::trace::SpanData); + } + + /** + * OnStart is called when a span starts; that span is added to running_spans. + * @param span a recordable for a span that was just started + */ + void OnStart(opentelemetry::sdk::trace::Recordable &span) noexcept override; + + /** + * OnEnd is called when a span ends; that span is moved from running_spans to + * completed_spans + * @param span a recordable for a span that was ended + */ + void OnEnd(std::unique_ptr &&span) noexcept override; + + /** + * Returns a snapshot of all spans stored. This snapshot has a copy of the + * stored running_spans and gives ownership of completed spans. Stored + * completed_spans are cleared from the processor. Currently, copy-on-write + * is utilized where possible to minimize contention, but locks may be added + * in the future. + * @return snapshot of all currently running spans and newly completed spans + * at the time that the function is called + */ + CollectedSpans GetSpanSnapshot() noexcept; + + /** + * Send all ended spans that have not yet been sent. + * @param timeout an optional timeout, the default timeout of 0 means that no + * timeout is applied. + */ + void ForceFlush( + std::chrono::microseconds timeout = std::chrono::microseconds(0)) noexcept override + { + if (shutdown_signal_received_) return; + std::this_thread::sleep_for(timeout); + // TODO: possibly send completed spans to data aggregator when called + } + /** + * Shut down the processor and do any cleanup required. Ended spans are + * send before shutdown. After the call to Shutdown, subsequent calls to + * OnStart, OnEnd, ForceFlush or Shutdown will return immediately without + * doing anything. + * @param timeout an optional timeout, the default timeout of 0 means that no + * timeout is applied. + */ + void Shutdown(std::chrono::microseconds timeout = std::chrono::microseconds(0)) noexcept override + { + ForceFlush(timeout); + shutdown_signal_received_ = true; + } + + private: + CollectedSpans spans_; + bool shutdown_signal_received_ = false; +}; +} // namespace zpages +} // namespace ext +OPENTELEMETRY_END_NAMESPACE diff --git a/ext/src/CMakeLists.txt b/ext/src/CMakeLists.txt new file mode 100644 index 0000000000..189a03f69c --- /dev/null +++ b/ext/src/CMakeLists.txt @@ -0,0 +1 @@ +add_subdirectory(zpages) diff --git a/ext/src/zpages/BUILD b/ext/src/zpages/BUILD new file mode 100644 index 0000000000..4b1a51df5c --- /dev/null +++ b/ext/src/zpages/BUILD @@ -0,0 +1,27 @@ +# 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", + "//sdk:headers", + "//ext:headers", + ], +) diff --git a/ext/src/zpages/CMakeLists.txt b/ext/src/zpages/CMakeLists.txt new file mode 100644 index 0000000000..d72ebd1a5d --- /dev/null +++ b/ext/src/zpages/CMakeLists.txt @@ -0,0 +1,4 @@ +add_library(opentelemetry_zpages tracez_processer.cc) + +target_link_libraries(opentelemetry_zpages ${CMAKE_THREAD_LIBS_INIT} + opentelemetry_trace) diff --git a/ext/src/zpages/tracez_processor.cc b/ext/src/zpages/tracez_processor.cc new file mode 100644 index 0000000000..3b4ab3ab24 --- /dev/null +++ b/ext/src/zpages/tracez_processor.cc @@ -0,0 +1,34 @@ +#include "opentelemetry/ext/zpages/tracez_processor.h" + +OPENTELEMETRY_BEGIN_NAMESPACE +namespace ext { +namespace zpages { + + void TracezSpanProcessor::OnStart(opentelemetry::sdk::trace::Recordable &span) noexcept { + if (shutdown_signal_received_) return; + spans_.running.insert(&span); + } + + void TracezSpanProcessor::OnEnd(std::unique_ptr &&span) noexcept { + if (shutdown_signal_received_ || span == nullptr) return; + auto completed_span = spans_.running.find(span.get()); + if (completed_span != spans_.running.end() && completed_span != nullptr) { + spans_.running.erase(completed_span); + spans_.completed.push_back(std::move(span)); + } + } + + + TracezSpanProcessor::CollectedSpans TracezSpanProcessor::GetSpanSnapshot() noexcept { + if (shutdown_signal_received_) return TracezSpanProcessor::CollectedSpans(); + CollectedSpans snapshot; + snapshot.running = spans_.running; + snapshot.completed = std::move(spans_.completed); + spans_.completed.clear(); + return snapshot; + } + + +} // namespace zpages +} // namespace ext +OPENTELEMETRY_END_NAMESPACE diff --git a/ext/test/CMakeLists.txt b/ext/test/CMakeLists.txt new file mode 100644 index 0000000000..189a03f69c --- /dev/null +++ b/ext/test/CMakeLists.txt @@ -0,0 +1 @@ +add_subdirectory(zpages) diff --git a/ext/test/zpages/BUILD b/ext/test/zpages/BUILD new file mode 100644 index 0000000000..50d027dbbd --- /dev/null +++ b/ext/test/zpages/BUILD @@ -0,0 +1,11 @@ +cc_test( + name = "tracez_processor_tests", + srcs = [ + "tracez_processor_test.cc", + ], + deps = [ + "//sdk/src/trace", + "//ext/src/zpages", + "@com_google_googletest//:gtest_main", + ], +) diff --git a/ext/test/zpages/CMakeLists.txt b/ext/test/zpages/CMakeLists.txt new file mode 100644 index 0000000000..ce275b1440 --- /dev/null +++ b/ext/test/zpages/CMakeLists.txt @@ -0,0 +1,16 @@ +foreach(testname + tracez_processor_test) + add_executable(${testname} "${testname}.cc") + target_link_libraries( + ${testname} ${GTEST_BOTH_LIBRARIES} ${CMAKE_THREAD_LIBS_INIT} + opentelemetry_zpages opentelemetry_trace) + + gtest_add_tests( + TARGET ${testname} + TEST_PREFIX ext. + TEST_LIST ${testname}) +endforeach() + +add_executable(tracez_processor_test tracez_processor_test.cc) +target_link_libraries(tracez_processor_test ${CMAKE_THREAD_LIBS_INIT} opentelemetry_zpages opentelemetry_trace) +add_test(tracez_processor_test) diff --git a/ext/test/zpages/tracez_processor_test.cc b/ext/test/zpages/tracez_processor_test.cc new file mode 100644 index 0000000000..a98078ef77 --- /dev/null +++ b/ext/test/zpages/tracez_processor_test.cc @@ -0,0 +1,464 @@ +#include "opentelemetry/ext/zpages/tracez_processor.h" +#include "opentelemetry/nostd/span.h" +#include "opentelemetry/sdk/trace/span_data.h" +#include "opentelemetry/sdk/trace/tracer.h" + +#include +#include + +using namespace opentelemetry::sdk::trace; +using namespace opentelemetry::ext::zpages; + + +// TODO: add tests for checking if spans are accurate when getting snapshots, +// like when a span completes mid getter call +// TODO: add tests with ForceFlush and Shutdown + +/* + * Helper function to create a processor, which is used while processor details + * are still being worked on + */ +std::shared_ptr MakeProcessor() { + std::shared_ptr processor(new TracezSpanProcessor()); + return processor; +} + + +/* + * Helper function uses the current processor tov update spans contained in completed_spans + * and running_spans. completed acts contains all spans, unless marked otherwise + */ +void UpdateSpans(std::shared_ptr& processor, + std::vector>& completed, + std::unordered_set& running, + bool store_only_new_completed = false) { + auto spans = processor->GetSpanSnapshot(); + running = spans.running; + if (store_only_new_completed) { + completed.clear(); + completed = std::move(spans.completed); + } + else { + std::move(spans.completed.begin(), spans.completed.end(), + std::inserter(completed, completed.end())); + } + spans.completed.clear(); + +} + + +/* + * Returns true if all the span names in the name vector within the given range appears in + * at least the same frequency as they do in running_spans. + * + * If no start value is given, start at index 0 + * If no end value is given, end at name vector end + * If 1-1 correspondance marked, return true if completed has all names in same frequency, no more or less + */ +bool ContainsNames(const std::vector& names, + std::unordered_set& running, + unsigned int name_start = 0, unsigned int name_end = 0, + bool one_to_one_correspondence = false) { + /* TEMPORARILY COMMENTED OUT WHILE RECORDABLE HAS NO GetName() FUNCTION + if (name_end == 0) name_end = names.size(); + + unsigned int num_names = name_end - name_start; + + if (num_names > running.size() || // More names than spans, can't have all names + (one_to_one_correspondence && num_names != running.size())) { + return false; + } + std::vector is_contained(num_names, false); + + // Mark all names that are contained only once + // in the order they appear + for (auto &span : running) { + for (unsigned int i = 0; i < num_names; i++) { + if (span->GetName() == names[name_start + i] && !is_contained[i]) { + is_contained[i] = true; + break; + } + } + } + + for (auto &&b : is_contained) if (!b) return false; + */ + return true; + +} + + +/* + * Returns true if all the span names in the nam vector within the given range appears in + * at least the same frequency as they do in completed_spans + * + * If no start value is given, start at index 0 + * If no end value is given, end at name vector end + * If 1-1 correspondance marked, return true if completed has all names in same frequency, no more or less + */ +bool ContainsNames(const std::vector& names, + std::vector>& completed, + unsigned int name_start = 0, unsigned int name_end = 0, + bool one_to_one_correspondence = false) { + /* TEMPORARILY COMMENTED OUT WHILE RECORDABLE HAS NO GetName() FUNCTION + + if (name_end == 0) name_end = names.size(); + + unsigned int num_names = name_end - name_start; + + if (num_names > completed.size() || + (one_to_one_correspondence && num_names != completed.size())) { + return false; + } + std::vector is_contained(num_names, false); + + for (auto &span : completed) { + for (unsigned int i = 0; i < num_names; i++) { + if (span->GetName() == names[name_start + i] && !is_contained[i]) { + is_contained[i] = true; + break; + } + } + } + + for (auto &&b : is_contained) if (!b) return false; + */ + return true; + +} + + +/* + * Test if both span containers are empty when no spans exist or are added + */ +TEST(TracezSpanProcessor, NoSpans) { + std::shared_ptr processor(new TracezSpanProcessor()); + auto spans = processor->GetSpanSnapshot(); + auto recordable = processor->MakeRecordable(); + + ASSERT_EQ(spans.running.size(), 0); + ASSERT_EQ(spans.running.size(), 0); + +} + + +/* + * Test if a single span moves from running to completed at expected times. + * All completed spans are stored. +*/ +TEST(TracezSpanProcessor, OneSpanRightContainerStored) { + std::shared_ptr processor(new TracezSpanProcessor()); + auto tracer = std::shared_ptr(new Tracer(processor)); + auto spans = processor->GetSpanSnapshot(); + auto running = spans.running; + auto completed = std::move(spans.completed); + + std::vector span_name = { "span" }; + + auto span = tracer->StartSpan(span_name[0]); + UpdateSpans(processor, completed, running); + + ASSERT_TRUE(ContainsNames(span_name, running)); + ASSERT_EQ(running.size(), 1); + ASSERT_EQ(completed.size(), 0); + + span->End(); + UpdateSpans(processor, completed, running); + + ASSERT_TRUE(ContainsNames(span_name, completed)); + ASSERT_EQ(running.size(), 0); + ASSERT_EQ(completed.size(), 1); + +} + + +/* + * Test if multiple spans move from running to completed at expected times. Check if + * all are in a container, either running/completed during checks. + * All completed spans are stored. +*/ +TEST(TracezSpanProcessor, MultipleSpansRightContainerStored) { + std::shared_ptr processor(new TracezSpanProcessor()); + auto tracer = std::shared_ptr(new Tracer(processor)); + auto spans = processor->GetSpanSnapshot(); + auto running = spans.running; + auto completed = std::move(spans.completed); + + ASSERT_EQ(running.size(), 0); + ASSERT_EQ(completed.size(), 0); + + std::vector span_names = {"s1", "s2", "s3", "s1"}; + + // Start and store spans using span_names + std::vector> span_vars; + for (const auto &name : span_names) span_vars.push_back(tracer->StartSpan(name)); + UpdateSpans(processor, completed, running); + + ASSERT_TRUE(ContainsNames(span_names, running)); // s1 s2 s3 s1 + ASSERT_EQ(running.size(), span_names.size()); + ASSERT_EQ(completed.size(), 0); + + // End all spans + for (auto &span : span_vars) span->End(); + UpdateSpans(processor, completed, running); + + ASSERT_TRUE(ContainsNames(span_names, completed)); // s1 s2 s3 s1 + ASSERT_EQ(running.size(), 0); + ASSERT_EQ(completed.size(), span_names.size()); + +} + + +/* + * Test if multiple spans move from running to completed at expected times, + * running/completed spans are split. Middle spans end first. All completed spans are stored. +*/ +TEST(TracezSpanProcessor, MultipleSpansRightContainerMiddleStored) { + std::shared_ptr processor(new TracezSpanProcessor()); + auto tracer = std::shared_ptr(new Tracer(processor)); + auto spans = processor->GetSpanSnapshot(); + auto running = spans.running; + auto completed = std::move(spans.completed); + + std::vector span_names = {"s0", "s2", "s1", "s1", "s"}; + + std::vector> span_vars; + for (const auto &name : span_names) span_vars.push_back(tracer->StartSpan(name)); + UpdateSpans(processor, completed, running); + + ASSERT_TRUE(ContainsNames(span_names, running)); // s0 s2 s1 s1 s + ASSERT_EQ(running.size(), span_names.size()); + ASSERT_EQ(completed.size(), 0); + + // End 4th span + span_vars[3]->End(); + UpdateSpans(processor, completed, running); + + ASSERT_TRUE(ContainsNames(span_names, running, 0, 3)); // s0 s2 s1 + ASSERT_TRUE(ContainsNames(span_names, running, 4)); // + s + ASSERT_TRUE(ContainsNames(span_names, completed, 3, 4)); // s1 + ASSERT_EQ(running.size(), 4); + ASSERT_EQ(completed.size(), 1); + + // End 2nd span + span_vars[1]->End(); + UpdateSpans(processor, completed, running); + + ASSERT_TRUE(ContainsNames(span_names, running, 0, 1)); // s0 + ASSERT_TRUE(ContainsNames(span_names, running, 2, 3)); // + s1 + ASSERT_TRUE(ContainsNames(span_names, running, 4)); // + s + ASSERT_TRUE(ContainsNames(span_names, completed, 1, 2)); // s2 + ASSERT_TRUE(ContainsNames(span_names, completed, 3, 4)); // s1 + ASSERT_EQ(running.size(), 3); + ASSERT_EQ(completed.size(), 2); + + // End 3rd span (last middle span) + span_vars[2]->End(); + UpdateSpans(processor, completed, running); + + ASSERT_TRUE(ContainsNames(span_names, running, 0, 1)); // s0 + ASSERT_TRUE(ContainsNames(span_names, running, 4)); // + s + ASSERT_TRUE(ContainsNames(span_names, completed, 1, 4)); // s2 s1 s1 + ASSERT_EQ(running.size(), 2); + ASSERT_EQ(completed.size(), 3); + + // End remaining Spans + span_vars[0]->End(); + span_vars[4]->End(); + UpdateSpans(processor, completed, running); + + ASSERT_TRUE(ContainsNames(span_names, completed)); // s0 s2 s1 s1 s + ASSERT_EQ(running.size(), 0); + ASSERT_EQ(completed.size(), 5); +} + + +/* + * Test if multiple spans move from running to completed at expected times, + * running/completed spans are split. All completed spans are stored. +*/ +TEST(TracezSpanProcessor, MultipleSpansRightContainerOuterStored) { + std::shared_ptr processor(new TracezSpanProcessor()); + auto tracer = std::shared_ptr(new Tracer(processor)); + auto spans = processor->GetSpanSnapshot(); + auto running = spans.running; + auto completed = std::move(spans.completed); + + std::vector span_names = {"s0", "s2", "s1", "s1", "s"}; + + std::vector> span_vars; + for (const auto &name : span_names) span_vars.push_back(tracer->StartSpan(name)); + + // End last span + span_vars[4]->End(); + UpdateSpans(processor, completed, running); + + ASSERT_TRUE(ContainsNames(span_names, running, 0, 4)); // s0 s2 s1 s1 + ASSERT_TRUE(ContainsNames(span_names, completed, 4)); // s + ASSERT_EQ(running.size(), 4); + ASSERT_EQ(completed.size(), 1); + + // End first span + span_vars[0]->End(); + UpdateSpans(processor, completed, running); + + ASSERT_TRUE(ContainsNames(span_names, running, 1, 4)); // s2 s1 s1 + ASSERT_TRUE(ContainsNames(span_names, completed, 0, 1)); // s0 + ASSERT_TRUE(ContainsNames(span_names, completed, 4)); // s + ASSERT_EQ(running.size(), 3); + ASSERT_EQ(completed.size(), 2); + + // End remaining Spans + for (int i = 1; i < 4; i++) span_vars[i]->End(); + UpdateSpans(processor, completed, running); + + ASSERT_TRUE(ContainsNames(span_names, completed)); // s0 s2 s1 s1 s + ASSERT_EQ(running.size(), 0); + ASSERT_EQ(completed.size(), 5); +} + +/* + * Test if a single span moves from running to completed at expected times. + * Only new completed spans are stored. +*/ +TEST(TracezSpanProcessor, OneSpanRightContainerNewOnly) { + std::shared_ptr processor(new TracezSpanProcessor()); + auto tracer = std::shared_ptr(new Tracer(processor)); + auto spans = processor->GetSpanSnapshot(); + auto running = spans.running; + auto completed = std::move(spans.completed); + + std::vector span_name = { "span" }; + + auto span = tracer->StartSpan(span_name[0]); + UpdateSpans(processor, completed, running, true); + + ASSERT_TRUE(ContainsNames(span_name, running, true)); + ASSERT_EQ(running.size(), 1); + ASSERT_EQ(completed.size(), 0); + + span->End(); + UpdateSpans(processor, completed, running, true); + + ASSERT_TRUE(ContainsNames(span_name, completed, true)); + ASSERT_EQ(running.size(), 0); + ASSERT_EQ(completed.size(), 1); + + UpdateSpans(processor, completed, running, true); + + ASSERT_TRUE(ContainsNames(span_name, completed, true)); + ASSERT_EQ(running.size(), 0); + ASSERT_EQ(completed.size(), 0); + +} + +/* + * Test if multiple spans move from running to completed at expected times, + * running/completed spans are split. Middle spans end first. Only new completed spans are stored. +*/ +TEST(TracezSpanProcessor, MultipleSpansRightContainerMiddleNewOnly) { + std::shared_ptr processor(new TracezSpanProcessor()); + auto tracer = std::shared_ptr(new Tracer(processor)); + auto spans = processor->GetSpanSnapshot(); + auto running = spans.running; + auto completed = std::move(spans.completed); + + std::vector span_names = {"s0", "s2", "s1", "s1", "s"}; + + std::vector> span_vars; + for (const auto &name : span_names) span_vars.push_back(tracer->StartSpan(name)); + UpdateSpans(processor, completed, running); + + ASSERT_TRUE(ContainsNames(span_names, running, true)); // s0 s2 s1 s1 s + ASSERT_EQ(running.size(), span_names.size()); + ASSERT_EQ(completed.size(), 0); + + // End 4th span + span_vars[3]->End(); + UpdateSpans(processor, completed, running, true); + + ASSERT_TRUE(ContainsNames(span_names, running, 0, 3)); // s0 s2 s1 + ASSERT_TRUE(ContainsNames(span_names, running, 4)); // + s + ASSERT_TRUE(ContainsNames(span_names, completed, 3, 4, true)); // s1 + ASSERT_EQ(running.size(), 4); + ASSERT_EQ(completed.size(), 1); + + // End 2nd and 3rd span + span_vars[1]->End(); + span_vars[2]->End(); + UpdateSpans(processor, completed, running, true); + + ASSERT_TRUE(ContainsNames(span_names, running, 0, 1)); // s0 + ASSERT_TRUE(ContainsNames(span_names, running, 4)); // + s + ASSERT_TRUE(ContainsNames(span_names, completed, 1, 3, true)); // s2 s1 + ASSERT_EQ(running.size(), 2); + ASSERT_EQ(completed.size(), 2); + + // End remaining Spans + span_vars[0]->End(); + span_vars[4]->End(); + UpdateSpans(processor, completed, running, true); + + ASSERT_TRUE(ContainsNames(span_names, completed, 0, 1)); // s0 + ASSERT_TRUE(ContainsNames(span_names, completed, 4)); // s + ASSERT_EQ(running.size(), 0); + ASSERT_EQ(completed.size(), 2); + + UpdateSpans(processor, completed, running, true); + + ASSERT_TRUE(ContainsNames(span_names, completed, true)); + ASSERT_EQ(running.size(), 0); + ASSERT_EQ(completed.size(), 0); +} + + +/* + * Test if multiple spans move from running to completed at expected times, + * running/completed spans are split. Only new completed spans are stored. +*/ +TEST(TracezSpanProcessor, MultipleSpansRightContainerOuterNewOnly) { + std::shared_ptr processor(new TracezSpanProcessor()); + auto tracer = std::shared_ptr(new Tracer(processor)); + auto spans = processor->GetSpanSnapshot(); + auto running = spans.running; + auto completed = std::move(spans.completed); + + std::vector span_names = {"s0", "s2", "s1", "s1", "s"}; + + std::vector> span_vars; + for (const auto &name : span_names) span_vars.push_back(tracer->StartSpan(name)); + + // End last span + span_vars[4]->End(); + UpdateSpans(processor, completed, running, true); + + ASSERT_TRUE(ContainsNames(span_names, running, 0, 4, true)); // s0 s2 s1 s1 + ASSERT_TRUE(ContainsNames(span_names, completed, 4, 5, true)); // s + ASSERT_EQ(running.size(), 4); + ASSERT_EQ(completed.size(), 1); + + // End first span + span_vars[0]->End(); + UpdateSpans(processor, completed, running, true); + + ASSERT_TRUE(ContainsNames(span_names, running, 1, 4, true)); // s2 s1 s1 + ASSERT_TRUE(ContainsNames(span_names, completed, 0, 1, true)); // s0 + ASSERT_EQ(running.size(), 3); + ASSERT_EQ(completed.size(), 1); + + // End remaining middle pans + for (int i = 1; i < 4; i++) span_vars[i]->End(); + UpdateSpans(processor, completed, running, true); + + ASSERT_TRUE(ContainsNames(span_names, completed, 1, 4, true)); // s2 s1 s1 + ASSERT_EQ(running.size(), 0); + ASSERT_EQ(completed.size(), 3); + + UpdateSpans(processor, completed, running, true); + + ASSERT_TRUE(ContainsNames(span_names, completed, true)); + ASSERT_EQ(running.size(), 0); + ASSERT_EQ(completed.size(), 0); +} + + From 8f3d1bc07fbf9153f758a823439a1890055bb5b4 Mon Sep 17 00:00:00 2001 From: Janet Vu Date: Wed, 8 Jul 2020 19:19:02 +0000 Subject: [PATCH 02/19] Change cmake files --- CMakeLists.txt | 1 + ext/src/zpages/CMakeLists.txt | 5 ++--- ext/src/zpages/tracez_processor.cc | 2 +- ext/test/zpages/CMakeLists.txt | 5 +---- 4 files changed, 5 insertions(+), 8 deletions(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index 9fbfc1c059..94b6c44354 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -56,3 +56,4 @@ add_subdirectory(sdk) include_directories(.) add_subdirectory(exporters) add_subdirectory(examples) +add_subdirectory(ext) diff --git a/ext/src/zpages/CMakeLists.txt b/ext/src/zpages/CMakeLists.txt index d72ebd1a5d..1e6ff7ba0f 100644 --- a/ext/src/zpages/CMakeLists.txt +++ b/ext/src/zpages/CMakeLists.txt @@ -1,4 +1,3 @@ -add_library(opentelemetry_zpages tracez_processer.cc) +add_library(opentelemetry_zpages tracez_processor.cc) -target_link_libraries(opentelemetry_zpages ${CMAKE_THREAD_LIBS_INIT} - opentelemetry_trace) +target_link_libraries(opentelemetry_zpages ${CMAKE_THREAD_LIBS_INIT} opentelemetry_api opentelemetry_trace) diff --git a/ext/src/zpages/tracez_processor.cc b/ext/src/zpages/tracez_processor.cc index 3b4ab3ab24..28bec9250c 100644 --- a/ext/src/zpages/tracez_processor.cc +++ b/ext/src/zpages/tracez_processor.cc @@ -12,7 +12,7 @@ namespace zpages { void TracezSpanProcessor::OnEnd(std::unique_ptr &&span) noexcept { if (shutdown_signal_received_ || span == nullptr) return; auto completed_span = spans_.running.find(span.get()); - if (completed_span != spans_.running.end() && completed_span != nullptr) { + if (completed_span != spans_.running.end()) { spans_.running.erase(completed_span); spans_.completed.push_back(std::move(span)); } diff --git a/ext/test/zpages/CMakeLists.txt b/ext/test/zpages/CMakeLists.txt index ce275b1440..c0e1ddc15b 100644 --- a/ext/test/zpages/CMakeLists.txt +++ b/ext/test/zpages/CMakeLists.txt @@ -3,7 +3,7 @@ foreach(testname add_executable(${testname} "${testname}.cc") target_link_libraries( ${testname} ${GTEST_BOTH_LIBRARIES} ${CMAKE_THREAD_LIBS_INIT} - opentelemetry_zpages opentelemetry_trace) + opentelemetry_zpages) gtest_add_tests( TARGET ${testname} @@ -11,6 +11,3 @@ foreach(testname TEST_LIST ${testname}) endforeach() -add_executable(tracez_processor_test tracez_processor_test.cc) -target_link_libraries(tracez_processor_test ${CMAKE_THREAD_LIBS_INIT} opentelemetry_zpages opentelemetry_trace) -add_test(tracez_processor_test) From f106309ad4d75a5113eeb4fdf1044186acceff9f Mon Sep 17 00:00:00 2001 From: Janet Vu Date: Wed, 8 Jul 2020 19:44:41 +0000 Subject: [PATCH 03/19] adjust cmake files --- ext/src/zpages/CMakeLists.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ext/src/zpages/CMakeLists.txt b/ext/src/zpages/CMakeLists.txt index 1e6ff7ba0f..787093e81d 100644 --- a/ext/src/zpages/CMakeLists.txt +++ b/ext/src/zpages/CMakeLists.txt @@ -1,3 +1,3 @@ add_library(opentelemetry_zpages tracez_processor.cc) -target_link_libraries(opentelemetry_zpages ${CMAKE_THREAD_LIBS_INIT} opentelemetry_api opentelemetry_trace) +target_link_libraries(opentelemetry_zpages opentelemetry_api opentelemetry_trace) From 75cb6eeedb94c01261353c4205767d273203f717 Mon Sep 17 00:00:00 2001 From: Janet Vu Date: Thu, 9 Jul 2020 18:19:50 +0000 Subject: [PATCH 04/19] Fix CMake and test style --- ext/src/zpages/CMakeLists.txt | 7 ++++++- ext/test/zpages/tracez_processor_test.cc | 13 +------------ 2 files changed, 7 insertions(+), 13 deletions(-) diff --git a/ext/src/zpages/CMakeLists.txt b/ext/src/zpages/CMakeLists.txt index 787093e81d..7d912f195e 100644 --- a/ext/src/zpages/CMakeLists.txt +++ b/ext/src/zpages/CMakeLists.txt @@ -1,3 +1,8 @@ -add_library(opentelemetry_zpages tracez_processor.cc) +add_library(opentelemetry_zpages + tracez_processor.cc + ../../include/opentelemetry/ext/zpages/tracez_processor.h) + +target_include_directories(opentelemetry_zpages PUBLIC ../../include) target_link_libraries(opentelemetry_zpages opentelemetry_api opentelemetry_trace) + diff --git a/ext/test/zpages/tracez_processor_test.cc b/ext/test/zpages/tracez_processor_test.cc index a98078ef77..7ec0349582 100644 --- a/ext/test/zpages/tracez_processor_test.cc +++ b/ext/test/zpages/tracez_processor_test.cc @@ -12,16 +12,6 @@ using namespace opentelemetry::ext::zpages; // TODO: add tests for checking if spans are accurate when getting snapshots, // like when a span completes mid getter call -// TODO: add tests with ForceFlush and Shutdown - -/* - * Helper function to create a processor, which is used while processor details - * are still being worked on - */ -std::shared_ptr MakeProcessor() { - std::shared_ptr processor(new TracezSpanProcessor()); - return processor; -} /* @@ -37,8 +27,7 @@ void UpdateSpans(std::shared_ptr& processor, if (store_only_new_completed) { completed.clear(); completed = std::move(spans.completed); - } - else { + } else { std::move(spans.completed.begin(), spans.completed.end(), std::inserter(completed, completed.end())); } From d5d8ed5ea91e6b50276be9d69feb783cea0bf573 Mon Sep 17 00:00:00 2001 From: Janet Vu Date: Thu, 9 Jul 2020 19:28:50 +0000 Subject: [PATCH 05/19] Add tests, better test names/docstrings --- ext/test/zpages/tracez_processor_test.cc | 144 ++++++++++++++++++++--- 1 file changed, 126 insertions(+), 18 deletions(-) diff --git a/ext/test/zpages/tracez_processor_test.cc b/ext/test/zpages/tracez_processor_test.cc index 7ec0349582..0d99923c4f 100644 --- a/ext/test/zpages/tracez_processor_test.cc +++ b/ext/test/zpages/tracez_processor_test.cc @@ -13,6 +13,22 @@ using namespace opentelemetry::ext::zpages; // TODO: add tests for checking if spans are accurate when getting snapshots, // like when a span completes mid getter call +/* + * Returns whether the times given are nearly the same + */ +bool AreAlmostEqual(std::chrono::microseconds t1, + std::chrono::microseconds t2) { + return t1 - t2 <= std::chrono::microseconds(0); +} + +/* + * Returns current time + timeout in microseconds + */ +std::chrono::microseconds GetTime(std::chrono::microseconds timeout = std::chrono::microseconds(0)) { + auto duration = std::chrono::system_clock::now().time_since_epoch(); + return std::chrono::duration_cast(duration) + timeout; +} + /* * Helper function uses the current processor tov update spans contained in completed_spans @@ -118,7 +134,8 @@ bool ContainsNames(const std::vector& names, /* - * Test if both span containers are empty when no spans exist or are added + * Test if both span containers are empty when no spans exist or are added. + * Ensures no rogue spans appear in the containers somehow. */ TEST(TracezSpanProcessor, NoSpans) { std::shared_ptr processor(new TracezSpanProcessor()); @@ -126,16 +143,17 @@ TEST(TracezSpanProcessor, NoSpans) { auto recordable = processor->MakeRecordable(); ASSERT_EQ(spans.running.size(), 0); - ASSERT_EQ(spans.running.size(), 0); + ASSERT_EQ(spans.completed.size(), 0); } /* * Test if a single span moves from running to completed at expected times. - * All completed spans are stored. + * All completed spans are stored. Ensures basic functionality and that accumulation + * can happen */ -TEST(TracezSpanProcessor, OneSpanRightContainerStored) { +TEST(TracezSpanProcessor, OneSpanCumulative) { std::shared_ptr processor(new TracezSpanProcessor()); auto tracer = std::shared_ptr(new Tracer(processor)); auto spans = processor->GetSpanSnapshot(); @@ -163,10 +181,11 @@ TEST(TracezSpanProcessor, OneSpanRightContainerStored) { /* * Test if multiple spans move from running to completed at expected times. Check if - * all are in a container, either running/completed during checks. + * all are in a container, either running/completed during checks. Ensures basic functionality + * and that accumulation can happen for many spans * All completed spans are stored. */ -TEST(TracezSpanProcessor, MultipleSpansRightContainerStored) { +TEST(TracezSpanProcessor, MultipleSpansCumulative) { std::shared_ptr processor(new TracezSpanProcessor()); auto tracer = std::shared_ptr(new Tracer(processor)); auto spans = processor->GetSpanSnapshot(); @@ -200,9 +219,11 @@ TEST(TracezSpanProcessor, MultipleSpansRightContainerStored) { /* * Test if multiple spans move from running to completed at expected times, - * running/completed spans are split. Middle spans end first. All completed spans are stored. + * running/completed spans are split. Middle spans end first. Ensures basic functionality + * and that accumulation can happen for many spans even spans that start and end non- + * sequentially. All completed spans are stored. */ -TEST(TracezSpanProcessor, MultipleSpansRightContainerMiddleStored) { +TEST(TracezSpanProcessor, MultipleSpansMiddleSplitCumulative) { std::shared_ptr processor(new TracezSpanProcessor()); auto tracer = std::shared_ptr(new Tracer(processor)); auto spans = processor->GetSpanSnapshot(); @@ -264,9 +285,11 @@ TEST(TracezSpanProcessor, MultipleSpansRightContainerMiddleStored) { /* * Test if multiple spans move from running to completed at expected times, - * running/completed spans are split. All completed spans are stored. + * running/completed spans are split. Ensures basic functionality and that + * accumulation can happen for many spans even spans that start and end non- + * sequentially. All completed spans are stored. */ -TEST(TracezSpanProcessor, MultipleSpansRightContainerOuterStored) { +TEST(TracezSpanProcessor, MultipleSpansOuterSplitCumulative) { std::shared_ptr processor(new TracezSpanProcessor()); auto tracer = std::shared_ptr(new Tracer(processor)); auto spans = processor->GetSpanSnapshot(); @@ -308,9 +331,10 @@ TEST(TracezSpanProcessor, MultipleSpansRightContainerOuterStored) { /* * Test if a single span moves from running to completed at expected times. - * Only new completed spans are stored. + * Ensure correct behavior even when spans are discarded. Only new completed + * spans are stored. */ -TEST(TracezSpanProcessor, OneSpanRightContainerNewOnly) { +TEST(TracezSpanProcessor, OneSpanNewOnly) { std::shared_ptr processor(new TracezSpanProcessor()); auto tracer = std::shared_ptr(new Tracer(processor)); auto spans = processor->GetSpanSnapshot(); @@ -343,9 +367,11 @@ TEST(TracezSpanProcessor, OneSpanRightContainerNewOnly) { /* * Test if multiple spans move from running to completed at expected times, - * running/completed spans are split. Middle spans end first. Only new completed spans are stored. -*/ -TEST(TracezSpanProcessor, MultipleSpansRightContainerMiddleNewOnly) { + * running/completed spans are split. Middle spans end first. Ensure correct + * behavior even when multiple spans are discarded, even when span starting and + * ending is non-sequential. Only new completed spans are stored. + */ +TEST(TracezSpanProcessor, MultipleSpansMiddleSplitNewOnly) { std::shared_ptr processor(new TracezSpanProcessor()); auto tracer = std::shared_ptr(new Tracer(processor)); auto spans = processor->GetSpanSnapshot(); @@ -402,10 +428,12 @@ TEST(TracezSpanProcessor, MultipleSpansRightContainerMiddleNewOnly) { /* - * Test if multiple spans move from running to completed at expected times, - * running/completed spans are split. Only new completed spans are stored. + * Test if multiple spans move from running to completed at expected times, + * running/completed spans are split. Ensure correct behavior even when + * multiple spans are discarded, even when span starting and ending is + * non-sequential. Only new completed spans are stored. */ -TEST(TracezSpanProcessor, MultipleSpansRightContainerOuterNewOnly) { +TEST(TracezSpanProcessor, MultipleSpansOuterSplitNewOnly) { std::shared_ptr processor(new TracezSpanProcessor()); auto tracer = std::shared_ptr(new Tracer(processor)); auto spans = processor->GetSpanSnapshot(); @@ -451,3 +479,83 @@ TEST(TracezSpanProcessor, MultipleSpansRightContainerOuterNewOnly) { } +/* + * Test if flush sleeps for approximately the correct duration when none is set. +*/ +TEST(TracezSpanProcessor, ForceFlushNoSleep) { + std::shared_ptr processor(new TracezSpanProcessor()); + auto goal_time = GetTime(); + processor->ForceFlush(); + + EXPECT_TRUE(AreAlmostEqual(goal_time, GetTime())); + +} + + +/* + * Test if flush sleeps for approximately the correct duration when short + * duration is set. +*/ +TEST(TracezSpanProcessor, ForceFlushSleep) { + std::shared_ptr processor(new TracezSpanProcessor()); + auto sleep_amount = std::chrono::microseconds(100); + auto goal_time = GetTime(sleep_amount); + processor->ForceFlush(sleep_amount); + + EXPECT_TRUE(AreAlmostEqual(goal_time, GetTime())); + +} + + +/* + * Test if shutdown works with no duration set +*/ +TEST(TracezSpanProcessor, ShutdownNoSleep) { + std::shared_ptr processor(new TracezSpanProcessor()); + auto tracer = std::shared_ptr(new Tracer(processor)); + auto spans = processor->GetSpanSnapshot(); + auto running = spans.running; + auto completed = std::move(spans.completed); + + // Shutdown + auto goal_time = GetTime(); + processor->Shutdown(); + + // Nothing should happen, functions return immediately + auto span = tracer->StartSpan("span"); + span->End(); + processor->ForceFlush(); + + UpdateSpans(processor, completed, running); + ASSERT_EQ(running.size(), 0); + ASSERT_EQ(completed.size(), 0); + + EXPECT_TRUE(AreAlmostEqual(goal_time, GetTime())); +} + + +/* + * Test if shutdown works with no duration set +*/ +TEST(TracezSpanProcessor, ShutdownSleep) { + std::shared_ptr processor(new TracezSpanProcessor()); + auto tracer = std::shared_ptr(new Tracer(processor)); + auto spans = processor->GetSpanSnapshot(); + auto running = spans.running; + auto completed = std::move(spans.completed); + + auto sleep_amount = std::chrono::microseconds(100); + auto goal_time = GetTime(sleep_amount); + processor->Shutdown(sleep_amount); + + auto span = tracer->StartSpan("span"); + span->End(); + processor->ForceFlush(); + + UpdateSpans(processor, completed, running); + ASSERT_EQ(running.size(), 0); + ASSERT_EQ(completed.size(), 0); + + EXPECT_TRUE(AreAlmostEqual(goal_time, GetTime())); +} + From f8df6bba16951566dd1408534467ef6bd1cefa21 Mon Sep 17 00:00:00 2001 From: Janet Vu Date: Fri, 10 Jul 2020 00:08:27 +0000 Subject: [PATCH 06/19] Cast recordable to span_data, fix test formatting --- .../ext/zpages/tracez_processor.h | 4 +-- ext/src/zpages/tracez_processor.cc | 22 ++++++++------- ext/test/zpages/tracez_processor_test.cc | 27 ++++++++----------- 3 files changed, 26 insertions(+), 27 deletions(-) diff --git a/ext/include/opentelemetry/ext/zpages/tracez_processor.h b/ext/include/opentelemetry/ext/zpages/tracez_processor.h index d1de94a52f..901d7337c3 100644 --- a/ext/include/opentelemetry/ext/zpages/tracez_processor.h +++ b/ext/include/opentelemetry/ext/zpages/tracez_processor.h @@ -27,8 +27,8 @@ class TracezSpanProcessor : public opentelemetry::sdk::trace::SpanProcessor { public: struct CollectedSpans { - std::unordered_set running; - std::vector> completed; + std::unordered_set running; + std::vector> completed; }; /** diff --git a/ext/src/zpages/tracez_processor.cc b/ext/src/zpages/tracez_processor.cc index 28bec9250c..8b85283325 100644 --- a/ext/src/zpages/tracez_processor.cc +++ b/ext/src/zpages/tracez_processor.cc @@ -6,25 +6,29 @@ namespace zpages { void TracezSpanProcessor::OnStart(opentelemetry::sdk::trace::Recordable &span) noexcept { if (shutdown_signal_received_) return; - spans_.running.insert(&span); + spans_.running.insert(static_cast(&span)); } void TracezSpanProcessor::OnEnd(std::unique_ptr &&span) noexcept { if (shutdown_signal_received_ || span == nullptr) return; - auto completed_span = spans_.running.find(span.get()); - if (completed_span != spans_.running.end()) { - spans_.running.erase(completed_span); - spans_.completed.push_back(std::move(span)); + auto completed_span_raw = static_cast(span.release()); + auto completed_span_it = spans_.running.find(completed_span_raw); + if (completed_span_it != spans_.running.end()) { + spans_.running.erase(completed_span_it); + spans_.completed.push_back( + std::unique_ptr( + completed_span_raw)); } } TracezSpanProcessor::CollectedSpans TracezSpanProcessor::GetSpanSnapshot() noexcept { - if (shutdown_signal_received_) return TracezSpanProcessor::CollectedSpans(); CollectedSpans snapshot; - snapshot.running = spans_.running; - snapshot.completed = std::move(spans_.completed); - spans_.completed.clear(); + if (!shutdown_signal_received_) { + snapshot.running = spans_.running; + snapshot.completed = std::move(spans_.completed); + spans_.completed.clear(); + } return snapshot; } diff --git a/ext/test/zpages/tracez_processor_test.cc b/ext/test/zpages/tracez_processor_test.cc index 0d99923c4f..a5298c7e17 100644 --- a/ext/test/zpages/tracez_processor_test.cc +++ b/ext/test/zpages/tracez_processor_test.cc @@ -16,8 +16,7 @@ using namespace opentelemetry::ext::zpages; /* * Returns whether the times given are nearly the same */ -bool AreAlmostEqual(std::chrono::microseconds t1, - std::chrono::microseconds t2) { +bool AreAlmostEqual(std::chrono::microseconds t1, std::chrono::microseconds t2) { return t1 - t2 <= std::chrono::microseconds(0); } @@ -31,12 +30,12 @@ std::chrono::microseconds GetTime(std::chrono::microseconds timeout = std::chron /* - * Helper function uses the current processor tov update spans contained in completed_spans - * and running_spans. completed acts contains all spans, unless marked otherwise + * Helper function uses the current processor to update spans contained in completed_spans + * and running_spans. completed_spans contains all spans (cumulative), unless marked otherwise */ void UpdateSpans(std::shared_ptr& processor, - std::vector>& completed, - std::unordered_set& running, + std::vector>& completed, + std::unordered_set& running, bool store_only_new_completed = false) { auto spans = processor->GetSpanSnapshot(); running = spans.running; @@ -61,10 +60,9 @@ void UpdateSpans(std::shared_ptr& processor, * If 1-1 correspondance marked, return true if completed has all names in same frequency, no more or less */ bool ContainsNames(const std::vector& names, - std::unordered_set& running, + std::unordered_set& running, unsigned int name_start = 0, unsigned int name_end = 0, bool one_to_one_correspondence = false) { - /* TEMPORARILY COMMENTED OUT WHILE RECORDABLE HAS NO GetName() FUNCTION if (name_end == 0) name_end = names.size(); unsigned int num_names = name_end - name_start; @@ -87,7 +85,7 @@ bool ContainsNames(const std::vector& names, } for (auto &&b : is_contained) if (!b) return false; - */ + return true; } @@ -102,10 +100,9 @@ bool ContainsNames(const std::vector& names, * If 1-1 correspondance marked, return true if completed has all names in same frequency, no more or less */ bool ContainsNames(const std::vector& names, - std::vector>& completed, + std::vector>& completed, unsigned int name_start = 0, unsigned int name_end = 0, bool one_to_one_correspondence = false) { - /* TEMPORARILY COMMENTED OUT WHILE RECORDABLE HAS NO GetName() FUNCTION if (name_end == 0) name_end = names.size(); @@ -127,7 +124,7 @@ bool ContainsNames(const std::vector& names, } for (auto &&b : is_contained) if (!b) return false; - */ + return true; } @@ -421,7 +418,6 @@ TEST(TracezSpanProcessor, MultipleSpansMiddleSplitNewOnly) { UpdateSpans(processor, completed, running, true); - ASSERT_TRUE(ContainsNames(span_names, completed, true)); ASSERT_EQ(running.size(), 0); ASSERT_EQ(completed.size(), 0); } @@ -473,7 +469,6 @@ TEST(TracezSpanProcessor, MultipleSpansOuterSplitNewOnly) { UpdateSpans(processor, completed, running, true); - ASSERT_TRUE(ContainsNames(span_names, completed, true)); ASSERT_EQ(running.size(), 0); ASSERT_EQ(completed.size(), 0); } @@ -535,7 +530,7 @@ TEST(TracezSpanProcessor, ShutdownNoSleep) { /* - * Test if shutdown works with no duration set + * Test if shutdown works with short duration set */ TEST(TracezSpanProcessor, ShutdownSleep) { std::shared_ptr processor(new TracezSpanProcessor()); @@ -550,7 +545,7 @@ TEST(TracezSpanProcessor, ShutdownSleep) { auto span = tracer->StartSpan("span"); span->End(); - processor->ForceFlush(); + processor->ForceFlush(sleep_amount); UpdateSpans(processor, completed, running); ASSERT_EQ(running.size(), 0); From 8508a00b37b85b707198f09fe7ca4e1506029692 Mon Sep 17 00:00:00 2001 From: Janet Vu Date: Fri, 10 Jul 2020 01:45:08 +0000 Subject: [PATCH 07/19] Add touchups to tests and formatting --- .../ext/zpages/tracez_processor.h | 50 +++++++++---------- ext/src/zpages/tracez_processor.cc | 3 +- ext/test/zpages/tracez_processor_test.cc | 24 +++------ 3 files changed, 33 insertions(+), 44 deletions(-) diff --git a/ext/include/opentelemetry/ext/zpages/tracez_processor.h b/ext/include/opentelemetry/ext/zpages/tracez_processor.h index 901d7337c3..45e4c88e19 100644 --- a/ext/include/opentelemetry/ext/zpages/tracez_processor.h +++ b/ext/include/opentelemetry/ext/zpages/tracez_processor.h @@ -6,7 +6,6 @@ #include #include #include -#include #include "opentelemetry/sdk/trace/recordable.h" #include "opentelemetry/sdk/trace/span_data.h" @@ -18,10 +17,9 @@ namespace ext { namespace zpages { -/** - * The span processor passes finished recordables to be used by the - * Data Aggregator to store spans before aggregating for TraceZ. - * +/* + * The span processor passes and stores running and completed recordables (casted as span_data) + * to be used by the TraceZ Data Aggregator. */ class TracezSpanProcessor : public opentelemetry::sdk::trace::SpanProcessor { public: @@ -31,13 +29,13 @@ class TracezSpanProcessor : public opentelemetry::sdk::trace::SpanProcessor { std::vector> completed; }; - /** + /* * Initialize a span processor. */ explicit TracezSpanProcessor() noexcept {} - /** - * Create a span recordable + /* + * Create a span recordable, which is span_data * @return a newly initialized recordable */ std::unique_ptr MakeRecordable() noexcept override @@ -45,32 +43,33 @@ class TracezSpanProcessor : public opentelemetry::sdk::trace::SpanProcessor { return std::unique_ptr(new opentelemetry::sdk::trace::SpanData); } - /** - * OnStart is called when a span starts; that span is added to running_spans. + /* + * OnStart is called when a span starts; the recordable is cast to span_data and added to running_spans. * @param span a recordable for a span that was just started */ void OnStart(opentelemetry::sdk::trace::Recordable &span) noexcept override; - /** - * OnEnd is called when a span ends; that span is moved from running_spans to + /* + * OnEnd is called when a span ends; that span_data is moved from running_spans to * completed_spans * @param span a recordable for a span that was ended */ void OnEnd(std::unique_ptr &&span) noexcept override; - /** + /* * Returns a snapshot of all spans stored. This snapshot has a copy of the - * stored running_spans and gives ownership of completed spans. Stored - * completed_spans are cleared from the processor. Currently, copy-on-write - * is utilized where possible to minimize contention, but locks may be added - * in the future. + * stored running_spans and gives ownership of completed spans to the caller. + * Stored completed_spans are cleared from the processor. Currently, + * copy-on-write is utilized where possible to minimize contention, but locks + * may be added in the future. * @return snapshot of all currently running spans and newly completed spans - * at the time that the function is called + * (spans never sent while complete) at the time that the function is called */ CollectedSpans GetSpanSnapshot() noexcept; - /** - * Send all ended spans that have not yet been sent. + /* + * For now, does nothing but sleep for the specified time. In the future, it + * may send all ended spans that have not yet been sent to the aggregator. * @param timeout an optional timeout, the default timeout of 0 means that no * timeout is applied. */ @@ -79,13 +78,12 @@ class TracezSpanProcessor : public opentelemetry::sdk::trace::SpanProcessor { { if (shutdown_signal_received_) return; std::this_thread::sleep_for(timeout); - // TODO: possibly send completed spans to data aggregator when called } - /** - * Shut down the processor and do any cleanup required. Ended spans are - * send before shutdown. After the call to Shutdown, subsequent calls to - * OnStart, OnEnd, ForceFlush or Shutdown will return immediately without - * doing anything. + + /* + * Shut down the processor and do any cleanup required through ForceFlush. + * After the call to Shutdown, subsequent calls to OnStart, OnEnd, ForceFlush + * or Shutdown will return immediately without doing anything. * @param timeout an optional timeout, the default timeout of 0 means that no * timeout is applied. */ diff --git a/ext/src/zpages/tracez_processor.cc b/ext/src/zpages/tracez_processor.cc index 8b85283325..057161a5f9 100644 --- a/ext/src/zpages/tracez_processor.cc +++ b/ext/src/zpages/tracez_processor.cc @@ -16,8 +16,7 @@ namespace zpages { if (completed_span_it != spans_.running.end()) { spans_.running.erase(completed_span_it); spans_.completed.push_back( - std::unique_ptr( - completed_span_raw)); + std::unique_ptr(completed_span_raw)); } } diff --git a/ext/test/zpages/tracez_processor_test.cc b/ext/test/zpages/tracez_processor_test.cc index a5298c7e17..af80dfed25 100644 --- a/ext/test/zpages/tracez_processor_test.cc +++ b/ext/test/zpages/tracez_processor_test.cc @@ -9,9 +9,7 @@ using namespace opentelemetry::sdk::trace; using namespace opentelemetry::ext::zpages; - -// TODO: add tests for checking if spans are accurate when getting snapshots, -// like when a span completes mid getter call +//////////////////////////////////// TEST HELPER FUNCTIONS /////////////////// /* * Returns whether the times given are nearly the same @@ -47,7 +45,6 @@ void UpdateSpans(std::shared_ptr& processor, std::inserter(completed, completed.end())); } spans.completed.clear(); - } @@ -87,7 +84,6 @@ bool ContainsNames(const std::vector& names, for (auto &&b : is_contained) if (!b) return false; return true; - } @@ -126,9 +122,9 @@ bool ContainsNames(const std::vector& names, for (auto &&b : is_contained) if (!b) return false; return true; - } +///////////////////////////////////////// TESTS ////////////////////////// /* * Test if both span containers are empty when no spans exist or are added. @@ -141,7 +137,6 @@ TEST(TracezSpanProcessor, NoSpans) { ASSERT_EQ(spans.running.size(), 0); ASSERT_EQ(spans.completed.size(), 0); - } @@ -172,7 +167,6 @@ TEST(TracezSpanProcessor, OneSpanCumulative) { ASSERT_TRUE(ContainsNames(span_name, completed)); ASSERT_EQ(running.size(), 0); ASSERT_EQ(completed.size(), 1); - } @@ -210,7 +204,6 @@ TEST(TracezSpanProcessor, MultipleSpansCumulative) { ASSERT_TRUE(ContainsNames(span_names, completed)); // s1 s2 s3 s1 ASSERT_EQ(running.size(), 0); ASSERT_EQ(completed.size(), span_names.size()); - } @@ -343,23 +336,21 @@ TEST(TracezSpanProcessor, OneSpanNewOnly) { auto span = tracer->StartSpan(span_name[0]); UpdateSpans(processor, completed, running, true); - ASSERT_TRUE(ContainsNames(span_name, running, true)); + ASSERT_TRUE(ContainsNames(span_name, running, 0, 1, true)); ASSERT_EQ(running.size(), 1); ASSERT_EQ(completed.size(), 0); span->End(); UpdateSpans(processor, completed, running, true); - ASSERT_TRUE(ContainsNames(span_name, completed, true)); + ASSERT_TRUE(ContainsNames(span_name, completed, 0, 1, true)); ASSERT_EQ(running.size(), 0); ASSERT_EQ(completed.size(), 1); UpdateSpans(processor, completed, running, true); - ASSERT_TRUE(ContainsNames(span_name, completed, true)); ASSERT_EQ(running.size(), 0); ASSERT_EQ(completed.size(), 0); - } /* @@ -381,7 +372,7 @@ TEST(TracezSpanProcessor, MultipleSpansMiddleSplitNewOnly) { for (const auto &name : span_names) span_vars.push_back(tracer->StartSpan(name)); UpdateSpans(processor, completed, running); - ASSERT_TRUE(ContainsNames(span_names, running, true)); // s0 s2 s1 s1 s + ASSERT_TRUE(ContainsNames(span_names, running, 0, 5, true)); // s0 s2 s1 s1 s ASSERT_EQ(running.size(), span_names.size()); ASSERT_EQ(completed.size(), 0); @@ -474,6 +465,9 @@ TEST(TracezSpanProcessor, MultipleSpansOuterSplitNewOnly) { } +// TODO: add tests for checking if spans are accurate when getting snapshots, +// like when a span completes mid getter call later on using threads + /* * Test if flush sleeps for approximately the correct duration when none is set. */ @@ -483,7 +477,6 @@ TEST(TracezSpanProcessor, ForceFlushNoSleep) { processor->ForceFlush(); EXPECT_TRUE(AreAlmostEqual(goal_time, GetTime())); - } @@ -498,7 +491,6 @@ TEST(TracezSpanProcessor, ForceFlushSleep) { processor->ForceFlush(sleep_amount); EXPECT_TRUE(AreAlmostEqual(goal_time, GetTime())); - } From f1f54f0ad476b223051948cb444e6703f8e61fc3 Mon Sep 17 00:00:00 2001 From: Janet Vu Date: Fri, 10 Jul 2020 16:45:33 +0000 Subject: [PATCH 08/19] Change asserts into expects --- ext/test/zpages/tracez_processor_test.cc | 190 +++++++++++------------ 1 file changed, 95 insertions(+), 95 deletions(-) diff --git a/ext/test/zpages/tracez_processor_test.cc b/ext/test/zpages/tracez_processor_test.cc index af80dfed25..552878fb75 100644 --- a/ext/test/zpages/tracez_processor_test.cc +++ b/ext/test/zpages/tracez_processor_test.cc @@ -135,8 +135,8 @@ TEST(TracezSpanProcessor, NoSpans) { auto spans = processor->GetSpanSnapshot(); auto recordable = processor->MakeRecordable(); - ASSERT_EQ(spans.running.size(), 0); - ASSERT_EQ(spans.completed.size(), 0); + EXPECT_EQ(spans.running.size(), 0); + EXPECT_EQ(spans.completed.size(), 0); } @@ -157,16 +157,16 @@ TEST(TracezSpanProcessor, OneSpanCumulative) { auto span = tracer->StartSpan(span_name[0]); UpdateSpans(processor, completed, running); - ASSERT_TRUE(ContainsNames(span_name, running)); - ASSERT_EQ(running.size(), 1); - ASSERT_EQ(completed.size(), 0); + EXPECT_TRUE(ContainsNames(span_name, running)); + EXPECT_EQ(running.size(), 1); + EXPECT_EQ(completed.size(), 0); span->End(); UpdateSpans(processor, completed, running); - ASSERT_TRUE(ContainsNames(span_name, completed)); - ASSERT_EQ(running.size(), 0); - ASSERT_EQ(completed.size(), 1); + EXPECT_TRUE(ContainsNames(span_name, completed)); + EXPECT_EQ(running.size(), 0); + EXPECT_EQ(completed.size(), 1); } @@ -183,8 +183,8 @@ TEST(TracezSpanProcessor, MultipleSpansCumulative) { auto running = spans.running; auto completed = std::move(spans.completed); - ASSERT_EQ(running.size(), 0); - ASSERT_EQ(completed.size(), 0); + EXPECT_EQ(running.size(), 0); + EXPECT_EQ(completed.size(), 0); std::vector span_names = {"s1", "s2", "s3", "s1"}; @@ -193,17 +193,17 @@ TEST(TracezSpanProcessor, MultipleSpansCumulative) { for (const auto &name : span_names) span_vars.push_back(tracer->StartSpan(name)); UpdateSpans(processor, completed, running); - ASSERT_TRUE(ContainsNames(span_names, running)); // s1 s2 s3 s1 - ASSERT_EQ(running.size(), span_names.size()); - ASSERT_EQ(completed.size(), 0); + EXPECT_TRUE(ContainsNames(span_names, running)); // s1 s2 s3 s1 + EXPECT_EQ(running.size(), span_names.size()); + EXPECT_EQ(completed.size(), 0); // End all spans for (auto &span : span_vars) span->End(); UpdateSpans(processor, completed, running); - ASSERT_TRUE(ContainsNames(span_names, completed)); // s1 s2 s3 s1 - ASSERT_EQ(running.size(), 0); - ASSERT_EQ(completed.size(), span_names.size()); + EXPECT_TRUE(ContainsNames(span_names, completed)); // s1 s2 s3 s1 + EXPECT_EQ(running.size(), 0); + EXPECT_EQ(completed.size(), span_names.size()); } @@ -226,50 +226,50 @@ TEST(TracezSpanProcessor, MultipleSpansMiddleSplitCumulative) { for (const auto &name : span_names) span_vars.push_back(tracer->StartSpan(name)); UpdateSpans(processor, completed, running); - ASSERT_TRUE(ContainsNames(span_names, running)); // s0 s2 s1 s1 s - ASSERT_EQ(running.size(), span_names.size()); - ASSERT_EQ(completed.size(), 0); + EXPECT_TRUE(ContainsNames(span_names, running)); // s0 s2 s1 s1 s + EXPECT_EQ(running.size(), span_names.size()); + EXPECT_EQ(completed.size(), 0); // End 4th span span_vars[3]->End(); UpdateSpans(processor, completed, running); - ASSERT_TRUE(ContainsNames(span_names, running, 0, 3)); // s0 s2 s1 - ASSERT_TRUE(ContainsNames(span_names, running, 4)); // + s - ASSERT_TRUE(ContainsNames(span_names, completed, 3, 4)); // s1 - ASSERT_EQ(running.size(), 4); - ASSERT_EQ(completed.size(), 1); + EXPECT_TRUE(ContainsNames(span_names, running, 0, 3)); // s0 s2 s1 + EXPECT_TRUE(ContainsNames(span_names, running, 4)); // + s + EXPECT_TRUE(ContainsNames(span_names, completed, 3, 4)); // s1 + EXPECT_EQ(running.size(), 4); + EXPECT_EQ(completed.size(), 1); // End 2nd span span_vars[1]->End(); UpdateSpans(processor, completed, running); - ASSERT_TRUE(ContainsNames(span_names, running, 0, 1)); // s0 - ASSERT_TRUE(ContainsNames(span_names, running, 2, 3)); // + s1 - ASSERT_TRUE(ContainsNames(span_names, running, 4)); // + s - ASSERT_TRUE(ContainsNames(span_names, completed, 1, 2)); // s2 - ASSERT_TRUE(ContainsNames(span_names, completed, 3, 4)); // s1 - ASSERT_EQ(running.size(), 3); - ASSERT_EQ(completed.size(), 2); + EXPECT_TRUE(ContainsNames(span_names, running, 0, 1)); // s0 + EXPECT_TRUE(ContainsNames(span_names, running, 2, 3)); // + s1 + EXPECT_TRUE(ContainsNames(span_names, running, 4)); // + s + EXPECT_TRUE(ContainsNames(span_names, completed, 1, 2)); // s2 + EXPECT_TRUE(ContainsNames(span_names, completed, 3, 4)); // s1 + EXPECT_EQ(running.size(), 3); + EXPECT_EQ(completed.size(), 2); // End 3rd span (last middle span) span_vars[2]->End(); UpdateSpans(processor, completed, running); - ASSERT_TRUE(ContainsNames(span_names, running, 0, 1)); // s0 - ASSERT_TRUE(ContainsNames(span_names, running, 4)); // + s - ASSERT_TRUE(ContainsNames(span_names, completed, 1, 4)); // s2 s1 s1 - ASSERT_EQ(running.size(), 2); - ASSERT_EQ(completed.size(), 3); + EXPECT_TRUE(ContainsNames(span_names, running, 0, 1)); // s0 + EXPECT_TRUE(ContainsNames(span_names, running, 4)); // + s + EXPECT_TRUE(ContainsNames(span_names, completed, 1, 4)); // s2 s1 s1 + EXPECT_EQ(running.size(), 2); + EXPECT_EQ(completed.size(), 3); // End remaining Spans span_vars[0]->End(); span_vars[4]->End(); UpdateSpans(processor, completed, running); - ASSERT_TRUE(ContainsNames(span_names, completed)); // s0 s2 s1 s1 s - ASSERT_EQ(running.size(), 0); - ASSERT_EQ(completed.size(), 5); + EXPECT_TRUE(ContainsNames(span_names, completed)); // s0 s2 s1 s1 s + EXPECT_EQ(running.size(), 0); + EXPECT_EQ(completed.size(), 5); } @@ -295,28 +295,28 @@ TEST(TracezSpanProcessor, MultipleSpansOuterSplitCumulative) { span_vars[4]->End(); UpdateSpans(processor, completed, running); - ASSERT_TRUE(ContainsNames(span_names, running, 0, 4)); // s0 s2 s1 s1 - ASSERT_TRUE(ContainsNames(span_names, completed, 4)); // s - ASSERT_EQ(running.size(), 4); - ASSERT_EQ(completed.size(), 1); + EXPECT_TRUE(ContainsNames(span_names, running, 0, 4)); // s0 s2 s1 s1 + EXPECT_TRUE(ContainsNames(span_names, completed, 4)); // s + EXPECT_EQ(running.size(), 4); + EXPECT_EQ(completed.size(), 1); // End first span span_vars[0]->End(); UpdateSpans(processor, completed, running); - ASSERT_TRUE(ContainsNames(span_names, running, 1, 4)); // s2 s1 s1 - ASSERT_TRUE(ContainsNames(span_names, completed, 0, 1)); // s0 - ASSERT_TRUE(ContainsNames(span_names, completed, 4)); // s - ASSERT_EQ(running.size(), 3); - ASSERT_EQ(completed.size(), 2); + EXPECT_TRUE(ContainsNames(span_names, running, 1, 4)); // s2 s1 s1 + EXPECT_TRUE(ContainsNames(span_names, completed, 0, 1)); // s0 + EXPECT_TRUE(ContainsNames(span_names, completed, 4)); // s + EXPECT_EQ(running.size(), 3); + EXPECT_EQ(completed.size(), 2); // End remaining Spans for (int i = 1; i < 4; i++) span_vars[i]->End(); UpdateSpans(processor, completed, running); - ASSERT_TRUE(ContainsNames(span_names, completed)); // s0 s2 s1 s1 s - ASSERT_EQ(running.size(), 0); - ASSERT_EQ(completed.size(), 5); + EXPECT_TRUE(ContainsNames(span_names, completed)); // s0 s2 s1 s1 s + EXPECT_EQ(running.size(), 0); + EXPECT_EQ(completed.size(), 5); } /* @@ -336,21 +336,21 @@ TEST(TracezSpanProcessor, OneSpanNewOnly) { auto span = tracer->StartSpan(span_name[0]); UpdateSpans(processor, completed, running, true); - ASSERT_TRUE(ContainsNames(span_name, running, 0, 1, true)); - ASSERT_EQ(running.size(), 1); - ASSERT_EQ(completed.size(), 0); + EXPECT_TRUE(ContainsNames(span_name, running, 0, 1, true)); + EXPECT_EQ(running.size(), 1); + EXPECT_EQ(completed.size(), 0); span->End(); UpdateSpans(processor, completed, running, true); - ASSERT_TRUE(ContainsNames(span_name, completed, 0, 1, true)); - ASSERT_EQ(running.size(), 0); - ASSERT_EQ(completed.size(), 1); + EXPECT_TRUE(ContainsNames(span_name, completed, 0, 1, true)); + EXPECT_EQ(running.size(), 0); + EXPECT_EQ(completed.size(), 1); UpdateSpans(processor, completed, running, true); - ASSERT_EQ(running.size(), 0); - ASSERT_EQ(completed.size(), 0); + EXPECT_EQ(running.size(), 0); + EXPECT_EQ(completed.size(), 0); } /* @@ -372,45 +372,45 @@ TEST(TracezSpanProcessor, MultipleSpansMiddleSplitNewOnly) { for (const auto &name : span_names) span_vars.push_back(tracer->StartSpan(name)); UpdateSpans(processor, completed, running); - ASSERT_TRUE(ContainsNames(span_names, running, 0, 5, true)); // s0 s2 s1 s1 s - ASSERT_EQ(running.size(), span_names.size()); - ASSERT_EQ(completed.size(), 0); + EXPECT_TRUE(ContainsNames(span_names, running, 0, 5, true)); // s0 s2 s1 s1 s + EXPECT_EQ(running.size(), span_names.size()); + EXPECT_EQ(completed.size(), 0); // End 4th span span_vars[3]->End(); UpdateSpans(processor, completed, running, true); - ASSERT_TRUE(ContainsNames(span_names, running, 0, 3)); // s0 s2 s1 - ASSERT_TRUE(ContainsNames(span_names, running, 4)); // + s - ASSERT_TRUE(ContainsNames(span_names, completed, 3, 4, true)); // s1 - ASSERT_EQ(running.size(), 4); - ASSERT_EQ(completed.size(), 1); + EXPECT_TRUE(ContainsNames(span_names, running, 0, 3)); // s0 s2 s1 + EXPECT_TRUE(ContainsNames(span_names, running, 4)); // + s + EXPECT_TRUE(ContainsNames(span_names, completed, 3, 4, true)); // s1 + EXPECT_EQ(running.size(), 4); + EXPECT_EQ(completed.size(), 1); // End 2nd and 3rd span span_vars[1]->End(); span_vars[2]->End(); UpdateSpans(processor, completed, running, true); - ASSERT_TRUE(ContainsNames(span_names, running, 0, 1)); // s0 - ASSERT_TRUE(ContainsNames(span_names, running, 4)); // + s - ASSERT_TRUE(ContainsNames(span_names, completed, 1, 3, true)); // s2 s1 - ASSERT_EQ(running.size(), 2); - ASSERT_EQ(completed.size(), 2); + EXPECT_TRUE(ContainsNames(span_names, running, 0, 1)); // s0 + EXPECT_TRUE(ContainsNames(span_names, running, 4)); // + s + EXPECT_TRUE(ContainsNames(span_names, completed, 1, 3, true)); // s2 s1 + EXPECT_EQ(running.size(), 2); + EXPECT_EQ(completed.size(), 2); // End remaining Spans span_vars[0]->End(); span_vars[4]->End(); UpdateSpans(processor, completed, running, true); - ASSERT_TRUE(ContainsNames(span_names, completed, 0, 1)); // s0 - ASSERT_TRUE(ContainsNames(span_names, completed, 4)); // s - ASSERT_EQ(running.size(), 0); - ASSERT_EQ(completed.size(), 2); + EXPECT_TRUE(ContainsNames(span_names, completed, 0, 1)); // s0 + EXPECT_TRUE(ContainsNames(span_names, completed, 4)); // s + EXPECT_EQ(running.size(), 0); + EXPECT_EQ(completed.size(), 2); UpdateSpans(processor, completed, running, true); - ASSERT_EQ(running.size(), 0); - ASSERT_EQ(completed.size(), 0); + EXPECT_EQ(running.size(), 0); + EXPECT_EQ(completed.size(), 0); } @@ -436,32 +436,32 @@ TEST(TracezSpanProcessor, MultipleSpansOuterSplitNewOnly) { span_vars[4]->End(); UpdateSpans(processor, completed, running, true); - ASSERT_TRUE(ContainsNames(span_names, running, 0, 4, true)); // s0 s2 s1 s1 - ASSERT_TRUE(ContainsNames(span_names, completed, 4, 5, true)); // s - ASSERT_EQ(running.size(), 4); - ASSERT_EQ(completed.size(), 1); + EXPECT_TRUE(ContainsNames(span_names, running, 0, 4, true)); // s0 s2 s1 s1 + EXPECT_TRUE(ContainsNames(span_names, completed, 4, 5, true)); // s + EXPECT_EQ(running.size(), 4); + EXPECT_EQ(completed.size(), 1); // End first span span_vars[0]->End(); UpdateSpans(processor, completed, running, true); - ASSERT_TRUE(ContainsNames(span_names, running, 1, 4, true)); // s2 s1 s1 - ASSERT_TRUE(ContainsNames(span_names, completed, 0, 1, true)); // s0 - ASSERT_EQ(running.size(), 3); - ASSERT_EQ(completed.size(), 1); + EXPECT_TRUE(ContainsNames(span_names, running, 1, 4, true)); // s2 s1 s1 + EXPECT_TRUE(ContainsNames(span_names, completed, 0, 1, true)); // s0 + EXPECT_EQ(running.size(), 3); + EXPECT_EQ(completed.size(), 1); // End remaining middle pans for (int i = 1; i < 4; i++) span_vars[i]->End(); UpdateSpans(processor, completed, running, true); - ASSERT_TRUE(ContainsNames(span_names, completed, 1, 4, true)); // s2 s1 s1 - ASSERT_EQ(running.size(), 0); - ASSERT_EQ(completed.size(), 3); + EXPECT_TRUE(ContainsNames(span_names, completed, 1, 4, true)); // s2 s1 s1 + EXPECT_EQ(running.size(), 0); + EXPECT_EQ(completed.size(), 3); UpdateSpans(processor, completed, running, true); - ASSERT_EQ(running.size(), 0); - ASSERT_EQ(completed.size(), 0); + EXPECT_EQ(running.size(), 0); + EXPECT_EQ(completed.size(), 0); } @@ -514,8 +514,8 @@ TEST(TracezSpanProcessor, ShutdownNoSleep) { processor->ForceFlush(); UpdateSpans(processor, completed, running); - ASSERT_EQ(running.size(), 0); - ASSERT_EQ(completed.size(), 0); + EXPECT_EQ(running.size(), 0); + EXPECT_EQ(completed.size(), 0); EXPECT_TRUE(AreAlmostEqual(goal_time, GetTime())); } @@ -540,8 +540,8 @@ TEST(TracezSpanProcessor, ShutdownSleep) { processor->ForceFlush(sleep_amount); UpdateSpans(processor, completed, running); - ASSERT_EQ(running.size(), 0); - ASSERT_EQ(completed.size(), 0); + EXPECT_EQ(running.size(), 0); + EXPECT_EQ(completed.size(), 0); EXPECT_TRUE(AreAlmostEqual(goal_time, GetTime())); } From 2b1a5b70d617aa22f7dd80a048cf52bea7127095 Mon Sep 17 00:00:00 2001 From: Janet Vu Date: Mon, 13 Jul 2020 13:01:25 +0000 Subject: [PATCH 09/19] Fix OnEnd mem leak, reset flush/shutdown functions --- .../ext/zpages/tracez_processor.h | 21 +--- ext/src/zpages/tracez_processor.cc | 22 ++-- ext/test/zpages/tracez_processor_test.cc | 103 +----------------- 3 files changed, 22 insertions(+), 124 deletions(-) diff --git a/ext/include/opentelemetry/ext/zpages/tracez_processor.h b/ext/include/opentelemetry/ext/zpages/tracez_processor.h index 45e4c88e19..b36e10c2f7 100644 --- a/ext/include/opentelemetry/ext/zpages/tracez_processor.h +++ b/ext/include/opentelemetry/ext/zpages/tracez_processor.h @@ -68,34 +68,25 @@ class TracezSpanProcessor : public opentelemetry::sdk::trace::SpanProcessor { CollectedSpans GetSpanSnapshot() noexcept; /* - * For now, does nothing but sleep for the specified time. In the future, it + * For now, does nothing. In the future, it * may send all ended spans that have not yet been sent to the aggregator. * @param timeout an optional timeout, the default timeout of 0 means that no - * timeout is applied. + * timeout is applied. Currently, timeout does nothing. */ void ForceFlush( - std::chrono::microseconds timeout = std::chrono::microseconds(0)) noexcept override - { - if (shutdown_signal_received_) return; - std::this_thread::sleep_for(timeout); - } + std::chrono::microseconds timeout = std::chrono::microseconds(0)) noexcept override {} /* - * Shut down the processor and do any cleanup required through ForceFlush. + * Shut down the processor and do any cleanup required, which is none. * After the call to Shutdown, subsequent calls to OnStart, OnEnd, ForceFlush * or Shutdown will return immediately without doing anything. * @param timeout an optional timeout, the default timeout of 0 means that no - * timeout is applied. + * timeout is applied. Currently, timeout does nothing. */ - void Shutdown(std::chrono::microseconds timeout = std::chrono::microseconds(0)) noexcept override - { - ForceFlush(timeout); - shutdown_signal_received_ = true; - } + void Shutdown(std::chrono::microseconds timeout = std::chrono::microseconds(0)) noexcept override {} private: CollectedSpans spans_; - bool shutdown_signal_received_ = false; }; } // namespace zpages } // namespace ext diff --git a/ext/src/zpages/tracez_processor.cc b/ext/src/zpages/tracez_processor.cc index 057161a5f9..ad4d59a1e8 100644 --- a/ext/src/zpages/tracez_processor.cc +++ b/ext/src/zpages/tracez_processor.cc @@ -5,29 +5,27 @@ namespace ext { namespace zpages { void TracezSpanProcessor::OnStart(opentelemetry::sdk::trace::Recordable &span) noexcept { - if (shutdown_signal_received_) return; spans_.running.insert(static_cast(&span)); } void TracezSpanProcessor::OnEnd(std::unique_ptr &&span) noexcept { - if (shutdown_signal_received_ || span == nullptr) return; - auto completed_span_raw = static_cast(span.release()); - auto completed_span_it = spans_.running.find(completed_span_raw); - if (completed_span_it != spans_.running.end()) { - spans_.running.erase(completed_span_it); + if (span == nullptr) return; + auto span_raw = static_cast(span.get()); + auto span_it = spans_.running.find(span_raw); + if (span_it != spans_.running.end()) { + spans_.running.erase(span_it); spans_.completed.push_back( - std::unique_ptr(completed_span_raw)); + std::unique_ptr( + static_cast(span.release()))); } } TracezSpanProcessor::CollectedSpans TracezSpanProcessor::GetSpanSnapshot() noexcept { CollectedSpans snapshot; - if (!shutdown_signal_received_) { - snapshot.running = spans_.running; - snapshot.completed = std::move(spans_.completed); - spans_.completed.clear(); - } + snapshot.running = spans_.running; + snapshot.completed = std::move(spans_.completed); + spans_.completed.clear(); return snapshot; } diff --git a/ext/test/zpages/tracez_processor_test.cc b/ext/test/zpages/tracez_processor_test.cc index 552878fb75..bb7029a8c2 100644 --- a/ext/test/zpages/tracez_processor_test.cc +++ b/ext/test/zpages/tracez_processor_test.cc @@ -9,23 +9,7 @@ using namespace opentelemetry::sdk::trace; using namespace opentelemetry::ext::zpages; -//////////////////////////////////// TEST HELPER FUNCTIONS /////////////////// - -/* - * Returns whether the times given are nearly the same - */ -bool AreAlmostEqual(std::chrono::microseconds t1, std::chrono::microseconds t2) { - return t1 - t2 <= std::chrono::microseconds(0); -} - -/* - * Returns current time + timeout in microseconds - */ -std::chrono::microseconds GetTime(std::chrono::microseconds timeout = std::chrono::microseconds(0)) { - auto duration = std::chrono::system_clock::now().time_since_epoch(); - return std::chrono::duration_cast(duration) + timeout; -} - +//////////////////////////////////// TEST HELPER FUNCTIONS ////////////////////////////// /* * Helper function uses the current processor to update spans contained in completed_spans @@ -54,7 +38,8 @@ void UpdateSpans(std::shared_ptr& processor, * * If no start value is given, start at index 0 * If no end value is given, end at name vector end - * If 1-1 correspondance marked, return true if completed has all names in same frequency, no more or less + * If 1-1 correspondance marked, return true if completed has all names in same frequency, + * no more or less */ bool ContainsNames(const std::vector& names, std::unordered_set& running, @@ -93,7 +78,8 @@ bool ContainsNames(const std::vector& names, * * If no start value is given, start at index 0 * If no end value is given, end at name vector end - * If 1-1 correspondance marked, return true if completed has all names in same frequency, no more or less + * If 1-1 correspondance marked, return true if completed has all names in same frequency, + * no more or less */ bool ContainsNames(const std::vector& names, std::vector>& completed, @@ -124,7 +110,7 @@ bool ContainsNames(const std::vector& names, return true; } -///////////////////////////////////////// TESTS ////////////////////////// +///////////////////////////////////////// TESTS /////////////////////////////////// /* * Test if both span containers are empty when no spans exist or are added. @@ -468,81 +454,4 @@ TEST(TracezSpanProcessor, MultipleSpansOuterSplitNewOnly) { // TODO: add tests for checking if spans are accurate when getting snapshots, // like when a span completes mid getter call later on using threads -/* - * Test if flush sleeps for approximately the correct duration when none is set. -*/ -TEST(TracezSpanProcessor, ForceFlushNoSleep) { - std::shared_ptr processor(new TracezSpanProcessor()); - auto goal_time = GetTime(); - processor->ForceFlush(); - - EXPECT_TRUE(AreAlmostEqual(goal_time, GetTime())); -} - - -/* - * Test if flush sleeps for approximately the correct duration when short - * duration is set. -*/ -TEST(TracezSpanProcessor, ForceFlushSleep) { - std::shared_ptr processor(new TracezSpanProcessor()); - auto sleep_amount = std::chrono::microseconds(100); - auto goal_time = GetTime(sleep_amount); - processor->ForceFlush(sleep_amount); - - EXPECT_TRUE(AreAlmostEqual(goal_time, GetTime())); -} - - -/* - * Test if shutdown works with no duration set -*/ -TEST(TracezSpanProcessor, ShutdownNoSleep) { - std::shared_ptr processor(new TracezSpanProcessor()); - auto tracer = std::shared_ptr(new Tracer(processor)); - auto spans = processor->GetSpanSnapshot(); - auto running = spans.running; - auto completed = std::move(spans.completed); - - // Shutdown - auto goal_time = GetTime(); - processor->Shutdown(); - - // Nothing should happen, functions return immediately - auto span = tracer->StartSpan("span"); - span->End(); - processor->ForceFlush(); - - UpdateSpans(processor, completed, running); - EXPECT_EQ(running.size(), 0); - EXPECT_EQ(completed.size(), 0); - - EXPECT_TRUE(AreAlmostEqual(goal_time, GetTime())); -} - - -/* - * Test if shutdown works with short duration set -*/ -TEST(TracezSpanProcessor, ShutdownSleep) { - std::shared_ptr processor(new TracezSpanProcessor()); - auto tracer = std::shared_ptr(new Tracer(processor)); - auto spans = processor->GetSpanSnapshot(); - auto running = spans.running; - auto completed = std::move(spans.completed); - - auto sleep_amount = std::chrono::microseconds(100); - auto goal_time = GetTime(sleep_amount); - processor->Shutdown(sleep_amount); - - auto span = tracer->StartSpan("span"); - span->End(); - processor->ForceFlush(sleep_amount); - - UpdateSpans(processor, completed, running); - EXPECT_EQ(running.size(), 0); - EXPECT_EQ(completed.size(), 0); - - EXPECT_TRUE(AreAlmostEqual(goal_time, GetTime())); -} From bc0aaeadd432ef9435c4d0978d887319546ba328 Mon Sep 17 00:00:00 2001 From: Janet Vu Date: Mon, 13 Jul 2020 16:33:58 +0000 Subject: [PATCH 10/19] Add TODOs for thread safety concerns, update README --- ext/src/zpages/README.md | 20 ++++++++++---------- ext/src/zpages/tracez_processor.cc | 2 ++ 2 files changed, 12 insertions(+), 10 deletions(-) diff --git a/ext/src/zpages/README.md b/ext/src/zpages/README.md index f1bad98516..702faf8671 100644 --- a/ext/src/zpages/README.md +++ b/ext/src/zpages/README.md @@ -1,5 +1,5 @@ # zPages -> Last updated 6/26/20 +> Last updated 7/13/20 # Table of Contents - [Summary](#summary) @@ -9,14 +9,14 @@ - [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. There are currently two types of zPages: TraceZ and RPCz. +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 and deadlocks. +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, zPages work by reading a process' spans using a SpanProcessor, which exports spans to the appropriate DataAggregator that a HttpServer uses. +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 @@ -24,11 +24,11 @@ On a high level, zPages work by reading a process' spans using a SpanProcessor, 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) - - Contact point for TraceZ to connect with a process, which collects tracing information and provides an interface for TDA. + - 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 to send the correct tracing information to the THS. + - 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 on all of the process's spans and 2) more detailed information on specific spans when clicked. + - 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). @@ -41,6 +41,6 @@ RPCz is a type of zPage that provides details on instrumented sent and received - [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) -- [Contribution Guidelines](https://github.com/open-telemetry/opentelemetry-cpp/blob/master/CONTRIBUTING.md) - - +- [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) diff --git a/ext/src/zpages/tracez_processor.cc b/ext/src/zpages/tracez_processor.cc index ad4d59a1e8..8d5ed8f5c0 100644 --- a/ext/src/zpages/tracez_processor.cc +++ b/ext/src/zpages/tracez_processor.cc @@ -8,6 +8,7 @@ namespace zpages { spans_.running.insert(static_cast(&span)); } + // TODO: Add thread safety measures void TracezSpanProcessor::OnEnd(std::unique_ptr &&span) noexcept { if (span == nullptr) return; auto span_raw = static_cast(span.get()); @@ -21,6 +22,7 @@ namespace zpages { } + // TODO: Add thread safety measures TracezSpanProcessor::CollectedSpans TracezSpanProcessor::GetSpanSnapshot() noexcept { CollectedSpans snapshot; snapshot.running = spans_.running; From af3b3a7fa75a0c51edf9d65216e358116a0fbfc8 Mon Sep 17 00:00:00 2001 From: Janet Vu Date: Mon, 13 Jul 2020 21:48:19 +0000 Subject: [PATCH 11/19] Make processor more thread-safe with tests --- .../ext/zpages/tracez_processor.h | 2 + ext/src/zpages/tracez_processor.cc | 23 +-- ext/test/zpages/tracez_processor_test.cc | 176 +++++++++++++++++- 3 files changed, 188 insertions(+), 13 deletions(-) diff --git a/ext/include/opentelemetry/ext/zpages/tracez_processor.h b/ext/include/opentelemetry/ext/zpages/tracez_processor.h index b36e10c2f7..010a637114 100644 --- a/ext/include/opentelemetry/ext/zpages/tracez_processor.h +++ b/ext/include/opentelemetry/ext/zpages/tracez_processor.h @@ -6,6 +6,7 @@ #include #include #include +#include #include "opentelemetry/sdk/trace/recordable.h" #include "opentelemetry/sdk/trace/span_data.h" @@ -86,6 +87,7 @@ class TracezSpanProcessor : public opentelemetry::sdk::trace::SpanProcessor { void Shutdown(std::chrono::microseconds timeout = std::chrono::microseconds(0)) noexcept override {} private: + mutable std::mutex mtx_; CollectedSpans spans_; }; } // namespace zpages diff --git a/ext/src/zpages/tracez_processor.cc b/ext/src/zpages/tracez_processor.cc index 8d5ed8f5c0..b76f49d72a 100644 --- a/ext/src/zpages/tracez_processor.cc +++ b/ext/src/zpages/tracez_processor.cc @@ -5,26 +5,27 @@ namespace ext { namespace zpages { void TracezSpanProcessor::OnStart(opentelemetry::sdk::trace::Recordable &span) noexcept { + std::lock_guard lock(mtx_); spans_.running.insert(static_cast(&span)); } - // TODO: Add thread safety measures void TracezSpanProcessor::OnEnd(std::unique_ptr &&span) noexcept { - if (span == nullptr) return; - auto span_raw = static_cast(span.get()); - auto span_it = spans_.running.find(span_raw); - if (span_it != spans_.running.end()) { - spans_.running.erase(span_it); - spans_.completed.push_back( - std::unique_ptr( - static_cast(span.release()))); - } + if (span == nullptr) return; + auto span_raw = static_cast(span.get()); + std::lock_guard lock(mtx_); + auto span_it = spans_.running.find(span_raw); + if (span_it != spans_.running.end()) { + spans_.running.erase(span_it); + spans_.completed.push_back( + std::unique_ptr( + static_cast(span.release()))); + } } - // TODO: Add thread safety measures TracezSpanProcessor::CollectedSpans TracezSpanProcessor::GetSpanSnapshot() noexcept { CollectedSpans snapshot; + std::lock_guard lock(mtx_); snapshot.running = spans_.running; snapshot.completed = std::move(spans_.completed); spans_.completed.clear(); diff --git a/ext/test/zpages/tracez_processor_test.cc b/ext/test/zpages/tracez_processor_test.cc index bb7029a8c2..c9bd47d8e3 100644 --- a/ext/test/zpages/tracez_processor_test.cc +++ b/ext/test/zpages/tracez_processor_test.cc @@ -5,6 +5,7 @@ #include #include +#include using namespace opentelemetry::sdk::trace; using namespace opentelemetry::ext::zpages; @@ -110,6 +111,41 @@ bool ContainsNames(const std::vector& names, return true; } + +/* + * Helper function calls GetSpanSnapshot() i times and does nothing with it + * otherwise. Used for testing thread safety + */ +void GetManySnapshots(std::shared_ptr& processor, int i) { + for (; i > 0; i--) processor->GetSpanSnapshot(); +} + + +/* + * Helper function that creates i spans, which are added into the passed + * in vector. Used for testing thread safety + */ +void StartManySpans(std::vector> &spans, + std::shared_ptr 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, checking + * i times. Used for testing thread safety + */ +void EndAllSpans(std::vector> &spans, + int i) { + unsigned int count = 0; + for (; i > 0; i--) { + for (unsigned int j = count; j < spans.size(); j++) { + spans[j]->End(); + } + count = spans.size(); + } +} + + ///////////////////////////////////////// TESTS /////////////////////////////////// /* @@ -451,7 +487,143 @@ TEST(TracezSpanProcessor, MultipleSpansOuterSplitNewOnly) { } -// TODO: add tests for checking if spans are accurate when getting snapshots, -// like when a span completes mid getter call later on using threads +/* + * Test if thread safety is compromised and errors are thrown when many spans + * start at the same time. Test is not failed if no errors occur. + */ +TEST(TracezSpanProcessor, RunningThreadSafety) { + std::shared_ptr processor(new TracezSpanProcessor()); + auto tracer = std::shared_ptr(new Tracer(processor)); + std::vector> spans; + + std::thread start1(StartManySpans, std::ref(spans), tracer, 5); + std::thread start2(StartManySpans, std::ref(spans), tracer, 5); + + start1.join(); + start2.join(); + + EndAllSpans(spans, 1); +} + + +/* + * Test if thread safety is compromised and errors are thrown when many spans + * end at the same time. Test is not failed if no errors occur. + */ +TEST(TracezSpanProcessor, CompletedThreadSafety) { + std::shared_ptr processor(new TracezSpanProcessor()); + auto tracer = std::shared_ptr(new Tracer(processor)); + std::vector> spans; + + StartManySpans(spans, tracer, 10); + + std::thread end1(EndAllSpans, std::ref(spans), 5); + std::thread end2(EndAllSpans, std::ref(spans), 5); + + end1.join(); + end2.join(); +} + + +/* + * Test if thread safety is compromised and errors are thrown when many + * snapshots are grabbed at the same time. Test is not failed if no + * errors occur. + */ +TEST(TracezSpanProcessor, SnapshotThreadSafety) { + std::shared_ptr processor(new TracezSpanProcessor()); + auto tracer = std::shared_ptr(new Tracer(processor)); + std::vector> spans; + + std::thread snap1(GetManySnapshots, std::ref(processor), 5); + std::thread snap2(GetManySnapshots, std::ref(processor), 5); + + snap1.join(); + snap2.join(); + + StartManySpans(spans, tracer, 10); + + std::thread snap3(GetManySnapshots, std::ref(processor), 5); + std::thread snap4(GetManySnapshots, std::ref(processor), 5); + + snap3.join(); + snap4.join(); + + EndAllSpans(spans, 1); +} + + +/* + * Test if thread safety is compromised and errors are thrown when many spans + * start while others are ending. Test is not failed if no errors occur. + */ +TEST(TracezSpanProcessor, RunningCompletedThreadSafety) { + std::shared_ptr processor(new TracezSpanProcessor()); + auto tracer = std::shared_ptr(new Tracer(processor)); + std::vector> spans; + + std::thread start(StartManySpans, std::ref(spans), tracer, 10); + std::thread end(EndAllSpans, std::ref(spans), 10); + + start.join(); + end.join(); +} + + +/* + * Test if thread safety is compromised and errors are thrown when many spans + * start while snapshots are being grabbed. Test is not failed if no errors occur. + */ +TEST(TracezSpanProcessor, RunningSnapshotThreadSafety) { + std::shared_ptr processor(new TracezSpanProcessor()); + auto tracer = std::shared_ptr(new Tracer(processor)); + std::vector> spans; + + std::thread start(StartManySpans, std::ref(spans), tracer, 10); + std::thread snapshots(GetManySnapshots, std::ref(processor), 10); + + start.join(); + snapshots.join(); + +} + + +/* + * Test if thread safety is compromised and errors are thrown when all spans + * end while snapshotd are being grabbed. Test is not failed if no errors occur. + */ +TEST(TracezSpanProcessor, SnapshotCompletedThreadSafety) { + std::shared_ptr processor(new TracezSpanProcessor()); + auto tracer = std::shared_ptr(new Tracer(processor)); + std::vector> spans; + + StartManySpans(spans, tracer, 10); + + std::thread snapshots(GetManySnapshots, std::ref(processor), 10); + std::thread end(EndAllSpans, std::ref(spans), 1); + + snapshots.join(); + end.join(); + +} + + +/* + * Test if thread safety is compromised and errors are thrown when many spans + * start and end while snapshots are being grabbed. Test is not failed if no errors occur. + */ +TEST(TracezSpanProcessor, RunningSnapshotCompletedThreadSafety) { + std::shared_ptr processor(new TracezSpanProcessor()); + auto tracer = std::shared_ptr(new Tracer(processor)); + std::vector> spans; + + std::thread start(StartManySpans, std::ref(spans), tracer, 10); + std::thread snapshots(GetManySnapshots, std::ref(processor), 10); + std::thread end(EndAllSpans, std::ref(spans), 10); + + start.join(); + snapshots.join(); + end.join(); +} From 7085288f2eb561bd6a920780427d3f15db7b4ea8 Mon Sep 17 00:00:00 2001 From: Janet Vu Date: Mon, 13 Jul 2020 22:01:13 +0000 Subject: [PATCH 12/19] Add missed TODO comment --- ext/src/zpages/tracez_processor.cc | 1 + 1 file changed, 1 insertion(+) diff --git a/ext/src/zpages/tracez_processor.cc b/ext/src/zpages/tracez_processor.cc index 8d5ed8f5c0..05714253eb 100644 --- a/ext/src/zpages/tracez_processor.cc +++ b/ext/src/zpages/tracez_processor.cc @@ -4,6 +4,7 @@ OPENTELEMETRY_BEGIN_NAMESPACE namespace ext { namespace zpages { + // TODO: Add thread safety measures void TracezSpanProcessor::OnStart(opentelemetry::sdk::trace::Recordable &span) noexcept { spans_.running.insert(static_cast(&span)); } From 09037be6234340d7e4bc0cb69d7dc852dcd22559 Mon Sep 17 00:00:00 2001 From: Janet Vu Date: Tue, 14 Jul 2020 11:31:56 +0000 Subject: [PATCH 13/19] Fix test thread safety, add test fixtures --- ext/src/zpages/tracez_processor.cc | 1 - ext/test/zpages/tracez_processor_test.cc | 249 +++++++++-------------- 2 files changed, 97 insertions(+), 153 deletions(-) diff --git a/ext/src/zpages/tracez_processor.cc b/ext/src/zpages/tracez_processor.cc index 90f407f2bf..b76f49d72a 100644 --- a/ext/src/zpages/tracez_processor.cc +++ b/ext/src/zpages/tracez_processor.cc @@ -4,7 +4,6 @@ OPENTELEMETRY_BEGIN_NAMESPACE namespace ext { namespace zpages { - // TODO: Add thread safety measures void TracezSpanProcessor::OnStart(opentelemetry::sdk::trace::Recordable &span) noexcept { std::lock_guard lock(mtx_); spans_.running.insert(static_cast(&span)); diff --git a/ext/test/zpages/tracez_processor_test.cc b/ext/test/zpages/tracez_processor_test.cc index c9bd47d8e3..f49e5d7d8d 100644 --- a/ext/test/zpages/tracez_processor_test.cc +++ b/ext/test/zpages/tracez_processor_test.cc @@ -10,6 +10,7 @@ using namespace opentelemetry::sdk::trace; using namespace opentelemetry::ext::zpages; + //////////////////////////////////// TEST HELPER FUNCTIONS ////////////////////////////// /* @@ -131,34 +132,55 @@ void StartManySpans(std::vector> &spans, - int i) { - unsigned int count = 0; - for (; i > 0; i--) { - for (unsigned int j = count; j < spans.size(); j++) { - spans[j]->End(); - } - count = spans.size(); - } +void EndAllSpans(std::vector> &spans) { + for (auto &span : spans) span->End(); } +//////////////////////////////// TEST FIXTURE ////////////////////////////////////// + +/* + * Reduce code duplication by having single area with shared setup code + */ +class TracezProcessor : public ::testing::Test { + protected: + void SetUp() override { + processor = std::shared_ptr(new TracezSpanProcessor()); + tracer = std::shared_ptr(new Tracer(processor)); + auto spans = processor->GetSpanSnapshot(); + running = spans.running; + completed = std::move(spans.completed); + + span_names = {"s0", "s2", "s1", "s1", "s"}; + + } + + std::shared_ptr processor; + std::shared_ptr tracer; + + std::unordered_set running; + std::vector> completed; + + std::vector span_names; + std::vector> span_vars; + +}; + + ///////////////////////////////////////// TESTS /////////////////////////////////// /* * Test if both span containers are empty when no spans exist or are added. * Ensures no rogue spans appear in the containers somehow. */ -TEST(TracezSpanProcessor, NoSpans) { - std::shared_ptr processor(new TracezSpanProcessor()); - auto spans = processor->GetSpanSnapshot(); +TEST_F(TracezProcessor, NoSpans) { auto recordable = processor->MakeRecordable(); - EXPECT_EQ(spans.running.size(), 0); - EXPECT_EQ(spans.completed.size(), 0); + EXPECT_EQ(running.size(), 0); + EXPECT_EQ(completed.size(), 0); } @@ -167,26 +189,18 @@ TEST(TracezSpanProcessor, NoSpans) { * All completed spans are stored. Ensures basic functionality and that accumulation * can happen */ -TEST(TracezSpanProcessor, OneSpanCumulative) { - std::shared_ptr processor(new TracezSpanProcessor()); - auto tracer = std::shared_ptr(new Tracer(processor)); - auto spans = processor->GetSpanSnapshot(); - auto running = spans.running; - auto completed = std::move(spans.completed); - - std::vector span_name = { "span" }; - - auto span = tracer->StartSpan(span_name[0]); +TEST_F(TracezProcessor, OneSpanCumulative) { + auto span = tracer->StartSpan(span_names[0]); UpdateSpans(processor, completed, running); - EXPECT_TRUE(ContainsNames(span_name, running)); + EXPECT_TRUE(ContainsNames(span_names, running, 0, 1, true)); EXPECT_EQ(running.size(), 1); EXPECT_EQ(completed.size(), 0); span->End(); UpdateSpans(processor, completed, running); - EXPECT_TRUE(ContainsNames(span_name, completed)); + EXPECT_TRUE(ContainsNames(span_names, completed, 0, 1, true)); EXPECT_EQ(running.size(), 0); EXPECT_EQ(completed.size(), 1); } @@ -198,24 +212,15 @@ TEST(TracezSpanProcessor, OneSpanCumulative) { * and that accumulation can happen for many spans * All completed spans are stored. */ -TEST(TracezSpanProcessor, MultipleSpansCumulative) { - std::shared_ptr processor(new TracezSpanProcessor()); - auto tracer = std::shared_ptr(new Tracer(processor)); - auto spans = processor->GetSpanSnapshot(); - auto running = spans.running; - auto completed = std::move(spans.completed); - +TEST_F(TracezProcessor, MultipleSpansCumulative) { EXPECT_EQ(running.size(), 0); EXPECT_EQ(completed.size(), 0); - std::vector span_names = {"s1", "s2", "s3", "s1"}; - // Start and store spans using span_names - std::vector> span_vars; for (const auto &name : span_names) span_vars.push_back(tracer->StartSpan(name)); UpdateSpans(processor, completed, running); - EXPECT_TRUE(ContainsNames(span_names, running)); // s1 s2 s3 s1 + EXPECT_TRUE(ContainsNames(span_names, running)); // s0 s2 s1 s1 s EXPECT_EQ(running.size(), span_names.size()); EXPECT_EQ(completed.size(), 0); @@ -223,7 +228,7 @@ TEST(TracezSpanProcessor, MultipleSpansCumulative) { for (auto &span : span_vars) span->End(); UpdateSpans(processor, completed, running); - EXPECT_TRUE(ContainsNames(span_names, completed)); // s1 s2 s3 s1 + EXPECT_TRUE(ContainsNames(span_names, completed)); // s0 s2 s1 s1 s EXPECT_EQ(running.size(), 0); EXPECT_EQ(completed.size(), span_names.size()); } @@ -235,16 +240,7 @@ TEST(TracezSpanProcessor, MultipleSpansCumulative) { * and that accumulation can happen for many spans even spans that start and end non- * sequentially. All completed spans are stored. */ -TEST(TracezSpanProcessor, MultipleSpansMiddleSplitCumulative) { - std::shared_ptr processor(new TracezSpanProcessor()); - auto tracer = std::shared_ptr(new Tracer(processor)); - auto spans = processor->GetSpanSnapshot(); - auto running = spans.running; - auto completed = std::move(spans.completed); - - std::vector span_names = {"s0", "s2", "s1", "s1", "s"}; - - std::vector> span_vars; +TEST_F(TracezProcessor, MultipleSpansMiddleSplitCumulative) { for (const auto &name : span_names) span_vars.push_back(tracer->StartSpan(name)); UpdateSpans(processor, completed, running); @@ -301,16 +297,7 @@ TEST(TracezSpanProcessor, MultipleSpansMiddleSplitCumulative) { * accumulation can happen for many spans even spans that start and end non- * sequentially. All completed spans are stored. */ -TEST(TracezSpanProcessor, MultipleSpansOuterSplitCumulative) { - std::shared_ptr processor(new TracezSpanProcessor()); - auto tracer = std::shared_ptr(new Tracer(processor)); - auto spans = processor->GetSpanSnapshot(); - auto running = spans.running; - auto completed = std::move(spans.completed); - - std::vector span_names = {"s0", "s2", "s1", "s1", "s"}; - - std::vector> span_vars; +TEST_F(TracezProcessor, MultipleSpansOuterSplitCumulative) { for (const auto &name : span_names) span_vars.push_back(tracer->StartSpan(name)); // End last span @@ -346,26 +333,18 @@ TEST(TracezSpanProcessor, MultipleSpansOuterSplitCumulative) { * Ensure correct behavior even when spans are discarded. Only new completed * spans are stored. */ -TEST(TracezSpanProcessor, OneSpanNewOnly) { - std::shared_ptr processor(new TracezSpanProcessor()); - auto tracer = std::shared_ptr(new Tracer(processor)); - auto spans = processor->GetSpanSnapshot(); - auto running = spans.running; - auto completed = std::move(spans.completed); - - std::vector span_name = { "span" }; - - auto span = tracer->StartSpan(span_name[0]); +TEST_F(TracezProcessor, OneSpanNewOnly) { + auto span = tracer->StartSpan(span_names[0]); UpdateSpans(processor, completed, running, true); - EXPECT_TRUE(ContainsNames(span_name, running, 0, 1, true)); + EXPECT_TRUE(ContainsNames(span_names, running, 0, 1, true)); EXPECT_EQ(running.size(), 1); EXPECT_EQ(completed.size(), 0); span->End(); UpdateSpans(processor, completed, running, true); - EXPECT_TRUE(ContainsNames(span_name, completed, 0, 1, true)); + EXPECT_TRUE(ContainsNames(span_names, completed, 0, 1, true)); EXPECT_EQ(running.size(), 0); EXPECT_EQ(completed.size(), 1); @@ -381,16 +360,7 @@ TEST(TracezSpanProcessor, OneSpanNewOnly) { * behavior even when multiple spans are discarded, even when span starting and * ending is non-sequential. Only new completed spans are stored. */ -TEST(TracezSpanProcessor, MultipleSpansMiddleSplitNewOnly) { - std::shared_ptr processor(new TracezSpanProcessor()); - auto tracer = std::shared_ptr(new Tracer(processor)); - auto spans = processor->GetSpanSnapshot(); - auto running = spans.running; - auto completed = std::move(spans.completed); - - std::vector span_names = {"s0", "s2", "s1", "s1", "s"}; - - std::vector> span_vars; +TEST_F(TracezProcessor, MultipleSpansMiddleSplitNewOnly) { for (const auto &name : span_names) span_vars.push_back(tracer->StartSpan(name)); UpdateSpans(processor, completed, running); @@ -442,16 +412,7 @@ TEST(TracezSpanProcessor, MultipleSpansMiddleSplitNewOnly) { * multiple spans are discarded, even when span starting and ending is * non-sequential. Only new completed spans are stored. */ -TEST(TracezSpanProcessor, MultipleSpansOuterSplitNewOnly) { - std::shared_ptr processor(new TracezSpanProcessor()); - auto tracer = std::shared_ptr(new Tracer(processor)); - auto spans = processor->GetSpanSnapshot(); - auto running = spans.running; - auto completed = std::move(spans.completed); - - std::vector span_names = {"s0", "s2", "s1", "s1", "s"}; - - std::vector> span_vars; +TEST_F(TracezProcessor, MultipleSpansOuterSplitNewOnly) { for (const auto &name : span_names) span_vars.push_back(tracer->StartSpan(name)); // End last span @@ -472,7 +433,7 @@ TEST(TracezSpanProcessor, MultipleSpansOuterSplitNewOnly) { EXPECT_EQ(running.size(), 3); EXPECT_EQ(completed.size(), 1); - // End remaining middle pans + // End remaining middle spans for (int i = 1; i < 4; i++) span_vars[i]->End(); UpdateSpans(processor, completed, running, true); @@ -488,37 +449,32 @@ TEST(TracezSpanProcessor, MultipleSpansOuterSplitNewOnly) { /* - * Test if thread safety is compromised and errors are thrown when many spans - * start at the same time. Test is not failed if no errors occur. + * Test for thread safety when many spans start at the same time. */ -TEST(TracezSpanProcessor, RunningThreadSafety) { - std::shared_ptr processor(new TracezSpanProcessor()); - auto tracer = std::shared_ptr(new Tracer(processor)); - std::vector> spans; +TEST_F(TracezProcessor, RunningThreadSafety) { + std::vector> spans1; + std::vector> spans2; - std::thread start1(StartManySpans, std::ref(spans), tracer, 5); - std::thread start2(StartManySpans, std::ref(spans), tracer, 5); + std::thread start1(StartManySpans, std::ref(spans1), tracer, 5); + std::thread start2(StartManySpans, std::ref(spans2), tracer, 5); start1.join(); start2.join(); - - EndAllSpans(spans, 1); } /* - * Test if thread safety is compromised and errors are thrown when many spans - * end at the same time. Test is not failed if no errors occur. + * Test for thread safety when many spans end at the same time */ -TEST(TracezSpanProcessor, CompletedThreadSafety) { - std::shared_ptr processor(new TracezSpanProcessor()); - auto tracer = std::shared_ptr(new Tracer(processor)); - std::vector> spans; +TEST_F(TracezProcessor, CompletedThreadSafety) { + std::vector> spans1; + std::vector> spans2; - StartManySpans(spans, tracer, 10); + StartManySpans(spans1, tracer, 5); + StartManySpans(spans2, tracer, 5); - std::thread end1(EndAllSpans, std::ref(spans), 5); - std::thread end2(EndAllSpans, std::ref(spans), 5); + std::thread end1(EndAllSpans, std::ref(spans1)); + std::thread end2(EndAllSpans, std::ref(spans2)); end1.join(); end2.join(); @@ -526,13 +482,9 @@ TEST(TracezSpanProcessor, CompletedThreadSafety) { /* - * Test if thread safety is compromised and errors are thrown when many - * snapshots are grabbed at the same time. Test is not failed if no - * errors occur. + * Test for thread safety when many snapshots are grabbed at the same time. */ -TEST(TracezSpanProcessor, SnapshotThreadSafety) { - std::shared_ptr processor(new TracezSpanProcessor()); - auto tracer = std::shared_ptr(new Tracer(processor)); +TEST_F(TracezProcessor, SnapshotThreadSafety) { std::vector> spans; std::thread snap1(GetManySnapshots, std::ref(processor), 5); @@ -541,7 +493,7 @@ TEST(TracezSpanProcessor, SnapshotThreadSafety) { snap1.join(); snap2.join(); - StartManySpans(spans, tracer, 10); + StartManySpans(spans, tracer, 5); std::thread snap3(GetManySnapshots, std::ref(processor), 5); std::thread snap4(GetManySnapshots, std::ref(processor), 5); @@ -549,21 +501,20 @@ TEST(TracezSpanProcessor, SnapshotThreadSafety) { snap3.join(); snap4.join(); - EndAllSpans(spans, 1); } /* - * Test if thread safety is compromised and errors are thrown when many spans - * start while others are ending. Test is not failed if no errors occur. + * Test for thread safety when many spans start while others are ending. */ -TEST(TracezSpanProcessor, RunningCompletedThreadSafety) { - std::shared_ptr processor(new TracezSpanProcessor()); - auto tracer = std::shared_ptr(new Tracer(processor)); - std::vector> spans; +TEST_F(TracezProcessor, RunningCompletedThreadSafety) { + std::vector> spans1; + std::vector> spans2; - std::thread start(StartManySpans, std::ref(spans), tracer, 10); - std::thread end(EndAllSpans, std::ref(spans), 10); + StartManySpans(spans1, tracer, 5); + + std::thread start(StartManySpans, std::ref(spans2), tracer, 5); + std::thread end(EndAllSpans, std::ref(spans1)); start.join(); end.join(); @@ -571,16 +522,13 @@ TEST(TracezSpanProcessor, RunningCompletedThreadSafety) { /* - * Test if thread safety is compromised and errors are thrown when many spans - * start while snapshots are being grabbed. Test is not failed if no errors occur. + * Test for thread safety when many span start while snapshots are being grabbed. */ -TEST(TracezSpanProcessor, RunningSnapshotThreadSafety) { - std::shared_ptr processor(new TracezSpanProcessor()); - auto tracer = std::shared_ptr(new Tracer(processor)); +TEST_F(TracezProcessor, RunningSnapshotThreadSafety) { std::vector> spans; - std::thread start(StartManySpans, std::ref(spans), tracer, 10); - std::thread snapshots(GetManySnapshots, std::ref(processor), 10); + std::thread start(StartManySpans, std::ref(spans), tracer, 5); + std::thread snapshots(GetManySnapshots, std::ref(processor), 5); start.join(); snapshots.join(); @@ -589,18 +537,15 @@ TEST(TracezSpanProcessor, RunningSnapshotThreadSafety) { /* - * Test if thread safety is compromised and errors are thrown when all spans - * end while snapshotd are being grabbed. Test is not failed if no errors occur. + * Test for thread safety when many spans end while snapshots are being grabbed. */ -TEST(TracezSpanProcessor, SnapshotCompletedThreadSafety) { - std::shared_ptr processor(new TracezSpanProcessor()); - auto tracer = std::shared_ptr(new Tracer(processor)); +TEST_F(TracezProcessor, SnapshotCompletedThreadSafety) { std::vector> spans; - StartManySpans(spans, tracer, 10); + StartManySpans(spans, tracer, 5); - std::thread snapshots(GetManySnapshots, std::ref(processor), 10); - std::thread end(EndAllSpans, std::ref(spans), 1); + std::thread snapshots(GetManySnapshots, std::ref(processor), 5); + std::thread end(EndAllSpans, std::ref(spans)); snapshots.join(); end.join(); @@ -609,17 +554,17 @@ TEST(TracezSpanProcessor, SnapshotCompletedThreadSafety) { /* - * Test if thread safety is compromised and errors are thrown when many spans - * start and end while snapshots are being grabbed. Test is not failed if no errors occur. + * Test for thread safety when many spans start and end while snapshots are being grabbed. */ -TEST(TracezSpanProcessor, RunningSnapshotCompletedThreadSafety) { - std::shared_ptr processor(new TracezSpanProcessor()); - auto tracer = std::shared_ptr(new Tracer(processor)); - std::vector> spans; +TEST_F(TracezProcessor, RunningSnapshotCompletedThreadSafety) { + std::vector> spans1; + std::vector> spans2; + + StartManySpans(spans1, tracer, 5); - std::thread start(StartManySpans, std::ref(spans), tracer, 10); - std::thread snapshots(GetManySnapshots, std::ref(processor), 10); - std::thread end(EndAllSpans, std::ref(spans), 10); + std::thread start(StartManySpans, std::ref(spans2), tracer, 5); + std::thread snapshots(GetManySnapshots, std::ref(processor), 5); + std::thread end(EndAllSpans, std::ref(spans1)); start.join(); snapshots.join(); From 69a30c915281069c10ab3f0d0e078ffc104d5e1c Mon Sep 17 00:00:00 2001 From: Janet Vu Date: Tue, 14 Jul 2020 12:45:12 +0000 Subject: [PATCH 14/19] Adjust import order, spacing --- .../ext/zpages/tracez_processor.h | 11 +++++----- ext/test/zpages/tracez_processor_test.cc | 21 +++++++------------ 2 files changed, 13 insertions(+), 19 deletions(-) diff --git a/ext/include/opentelemetry/ext/zpages/tracez_processor.h b/ext/include/opentelemetry/ext/zpages/tracez_processor.h index 010a637114..ce62da45eb 100644 --- a/ext/include/opentelemetry/ext/zpages/tracez_processor.h +++ b/ext/include/opentelemetry/ext/zpages/tracez_processor.h @@ -1,17 +1,16 @@ #pragma once -#include +#include "opentelemetry/sdk/trace/processor.h" + +#include +#include #include +#include #include #include -#include -#include -#include #include "opentelemetry/sdk/trace/recordable.h" #include "opentelemetry/sdk/trace/span_data.h" -#include "opentelemetry/sdk/trace/exporter.h" -#include "opentelemetry/sdk/trace/processor.h" OPENTELEMETRY_BEGIN_NAMESPACE namespace ext diff --git a/ext/test/zpages/tracez_processor_test.cc b/ext/test/zpages/tracez_processor_test.cc index f49e5d7d8d..ddcc33e718 100644 --- a/ext/test/zpages/tracez_processor_test.cc +++ b/ext/test/zpages/tracez_processor_test.cc @@ -1,16 +1,16 @@ #include "opentelemetry/ext/zpages/tracez_processor.h" -#include "opentelemetry/nostd/span.h" -#include "opentelemetry/sdk/trace/span_data.h" -#include "opentelemetry/sdk/trace/tracer.h" #include -#include + #include +#include "opentelemetry/nostd/span.h" +#include "opentelemetry/sdk/trace/span_data.h" +#include "opentelemetry/sdk/trace/tracer.h" + using namespace opentelemetry::sdk::trace; using namespace opentelemetry::ext::zpages; - //////////////////////////////////// TEST HELPER FUNCTIONS ////////////////////////////// /* @@ -131,6 +131,7 @@ void StartManySpans(std::vector 0; i--) spans.push_back(tracer->StartSpan("span")); } + /* * Helper function that ends all spans in the passed in span vector. Used * for testing thread safety @@ -328,6 +329,7 @@ TEST_F(TracezProcessor, MultipleSpansOuterSplitCumulative) { EXPECT_EQ(completed.size(), 5); } + /* * Test if a single span moves from running to completed at expected times. * Ensure correct behavior even when spans are discarded. Only new completed @@ -354,6 +356,7 @@ TEST_F(TracezProcessor, OneSpanNewOnly) { EXPECT_EQ(completed.size(), 0); } + /* * Test if multiple spans move from running to completed at expected times, * running/completed spans are split. Middle spans end first. Ensure correct @@ -469,7 +472,6 @@ TEST_F(TracezProcessor, RunningThreadSafety) { TEST_F(TracezProcessor, CompletedThreadSafety) { std::vector> spans1; std::vector> spans2; - StartManySpans(spans1, tracer, 5); StartManySpans(spans2, tracer, 5); @@ -500,7 +502,6 @@ TEST_F(TracezProcessor, SnapshotThreadSafety) { snap3.join(); snap4.join(); - } @@ -510,7 +511,6 @@ TEST_F(TracezProcessor, SnapshotThreadSafety) { TEST_F(TracezProcessor, RunningCompletedThreadSafety) { std::vector> spans1; std::vector> spans2; - StartManySpans(spans1, tracer, 5); std::thread start(StartManySpans, std::ref(spans2), tracer, 5); @@ -532,7 +532,6 @@ TEST_F(TracezProcessor, RunningSnapshotThreadSafety) { start.join(); snapshots.join(); - } @@ -541,7 +540,6 @@ TEST_F(TracezProcessor, RunningSnapshotThreadSafety) { */ TEST_F(TracezProcessor, SnapshotCompletedThreadSafety) { std::vector> spans; - StartManySpans(spans, tracer, 5); std::thread snapshots(GetManySnapshots, std::ref(processor), 5); @@ -549,7 +547,6 @@ TEST_F(TracezProcessor, SnapshotCompletedThreadSafety) { snapshots.join(); end.join(); - } @@ -559,7 +556,6 @@ TEST_F(TracezProcessor, SnapshotCompletedThreadSafety) { TEST_F(TracezProcessor, RunningSnapshotCompletedThreadSafety) { std::vector> spans1; std::vector> spans2; - StartManySpans(spans1, tracer, 5); std::thread start(StartManySpans, std::ref(spans2), tracer, 5); @@ -571,4 +567,3 @@ TEST_F(TracezProcessor, RunningSnapshotCompletedThreadSafety) { end.join(); } - From 466ac2e6e387b5a20a7455607392f7bf777b3f3c Mon Sep 17 00:00:00 2001 From: Janet Vu Date: Tue, 14 Jul 2020 14:53:17 +0000 Subject: [PATCH 15/19] Add noop tests for ForceFlush/Shutdown --- ext/test/zpages/tracez_processor_test.cc | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/ext/test/zpages/tracez_processor_test.cc b/ext/test/zpages/tracez_processor_test.cc index ddcc33e718..d3c2580209 100644 --- a/ext/test/zpages/tracez_processor_test.cc +++ b/ext/test/zpages/tracez_processor_test.cc @@ -451,6 +451,23 @@ TEST_F(TracezProcessor, MultipleSpansOuterSplitNewOnly) { } +/* + * Test for ForceFlush and Shutdown code coverage, which do nothing. + */ +TEST_F(TracezProcessor, FlushShutdown) { + auto pre_running_sz = running.size(); + auto pre_completed_sz = completed.size(); + + processor->ForceFlush(); + processor->Shutdown(); + + UpdateSpans(processor, completed, running); + + EXPECT_EQ(pre_running_sz, running.size()); + EXPECT_EQ(pre_completed_sz, completed.size()); +} + + /* * Test for thread safety when many spans start at the same time. */ From d9ba5b30e52b30d76e2642a2d0ec969f95e0df29 Mon Sep 17 00:00:00 2001 From: Janet Vu Date: Tue, 14 Jul 2020 15:34:50 +0000 Subject: [PATCH 16/19] Move includes, remove README update date --- ext/include/opentelemetry/ext/zpages/tracez_processor.h | 3 +-- ext/src/zpages/README.md | 3 +-- 2 files changed, 2 insertions(+), 4 deletions(-) diff --git a/ext/include/opentelemetry/ext/zpages/tracez_processor.h b/ext/include/opentelemetry/ext/zpages/tracez_processor.h index ce62da45eb..af793ce0f3 100644 --- a/ext/include/opentelemetry/ext/zpages/tracez_processor.h +++ b/ext/include/opentelemetry/ext/zpages/tracez_processor.h @@ -1,7 +1,5 @@ #pragma once -#include "opentelemetry/sdk/trace/processor.h" - #include #include #include @@ -9,6 +7,7 @@ #include #include +#include "opentelemetry/sdk/trace/processor.h" #include "opentelemetry/sdk/trace/recordable.h" #include "opentelemetry/sdk/trace/span_data.h" diff --git a/ext/src/zpages/README.md b/ext/src/zpages/README.md index 702faf8671..8e8fed394a 100644 --- a/ext/src/zpages/README.md +++ b/ext/src/zpages/README.md @@ -1,7 +1,6 @@ # zPages -> Last updated 7/13/20 -# Table of Contents +## Table of Contents - [Summary](#summary) - [TraceZ](#tracez) - [RPCz](#rpcz) From 0dc7aa669df2eb74787da0bf1d9b7bcde84b0784 Mon Sep 17 00:00:00 2001 From: Janet Vu Date: Tue, 14 Jul 2020 20:10:43 +0000 Subject: [PATCH 17/19] Order includes alphabetically --- ext/include/opentelemetry/ext/zpages/tracez_processor.h | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/ext/include/opentelemetry/ext/zpages/tracez_processor.h b/ext/include/opentelemetry/ext/zpages/tracez_processor.h index af793ce0f3..aca13d72fa 100644 --- a/ext/include/opentelemetry/ext/zpages/tracez_processor.h +++ b/ext/include/opentelemetry/ext/zpages/tracez_processor.h @@ -1,11 +1,11 @@ #pragma once -#include -#include -#include #include +#include +#include #include #include +#include #include "opentelemetry/sdk/trace/processor.h" #include "opentelemetry/sdk/trace/recordable.h" From dde9a4fda4052f315725a812974f286ee5f1345f Mon Sep 17 00:00:00 2001 From: Janet Vu Date: Wed, 15 Jul 2020 18:51:17 +0000 Subject: [PATCH 18/19] Increase span creation numbers --- ext/test/zpages/tracez_processor_test.cc | 38 ++++++++++++------------ 1 file changed, 19 insertions(+), 19 deletions(-) diff --git a/ext/test/zpages/tracez_processor_test.cc b/ext/test/zpages/tracez_processor_test.cc index d3c2580209..28fa4edc74 100644 --- a/ext/test/zpages/tracez_processor_test.cc +++ b/ext/test/zpages/tracez_processor_test.cc @@ -462,7 +462,7 @@ TEST_F(TracezProcessor, FlushShutdown) { processor->Shutdown(); UpdateSpans(processor, completed, running); - + EXPECT_EQ(pre_running_sz, running.size()); EXPECT_EQ(pre_completed_sz, completed.size()); } @@ -475,8 +475,8 @@ TEST_F(TracezProcessor, RunningThreadSafety) { std::vector> spans1; std::vector> spans2; - std::thread start1(StartManySpans, std::ref(spans1), tracer, 5); - std::thread start2(StartManySpans, std::ref(spans2), tracer, 5); + std::thread start1(StartManySpans, std::ref(spans1), tracer, 2000); + std::thread start2(StartManySpans, std::ref(spans2), tracer, 2000); start1.join(); start2.join(); @@ -489,8 +489,8 @@ TEST_F(TracezProcessor, RunningThreadSafety) { TEST_F(TracezProcessor, CompletedThreadSafety) { std::vector> spans1; std::vector> spans2; - StartManySpans(spans1, tracer, 5); - StartManySpans(spans2, tracer, 5); + StartManySpans(spans1, tracer, 2000); + StartManySpans(spans2, tracer, 2000); std::thread end1(EndAllSpans, std::ref(spans1)); std::thread end2(EndAllSpans, std::ref(spans2)); @@ -506,16 +506,16 @@ TEST_F(TracezProcessor, CompletedThreadSafety) { TEST_F(TracezProcessor, SnapshotThreadSafety) { std::vector> spans; - std::thread snap1(GetManySnapshots, std::ref(processor), 5); - std::thread snap2(GetManySnapshots, std::ref(processor), 5); + std::thread snap1(GetManySnapshots, std::ref(processor), 2000); + std::thread snap2(GetManySnapshots, std::ref(processor), 2000); snap1.join(); snap2.join(); - StartManySpans(spans, tracer, 5); + StartManySpans(spans, tracer, 2000); - std::thread snap3(GetManySnapshots, std::ref(processor), 5); - std::thread snap4(GetManySnapshots, std::ref(processor), 5); + std::thread snap3(GetManySnapshots, std::ref(processor), 2000); + std::thread snap4(GetManySnapshots, std::ref(processor), 2000); snap3.join(); snap4.join(); @@ -528,9 +528,9 @@ TEST_F(TracezProcessor, SnapshotThreadSafety) { TEST_F(TracezProcessor, RunningCompletedThreadSafety) { std::vector> spans1; std::vector> spans2; - StartManySpans(spans1, tracer, 5); + StartManySpans(spans1, tracer, 2000); - std::thread start(StartManySpans, std::ref(spans2), tracer, 5); + std::thread start(StartManySpans, std::ref(spans2), tracer, 2000); std::thread end(EndAllSpans, std::ref(spans1)); start.join(); @@ -544,8 +544,8 @@ TEST_F(TracezProcessor, RunningCompletedThreadSafety) { TEST_F(TracezProcessor, RunningSnapshotThreadSafety) { std::vector> spans; - std::thread start(StartManySpans, std::ref(spans), tracer, 5); - std::thread snapshots(GetManySnapshots, std::ref(processor), 5); + std::thread start(StartManySpans, std::ref(spans), tracer, 2000); + std::thread snapshots(GetManySnapshots, std::ref(processor), 2000); start.join(); snapshots.join(); @@ -557,9 +557,9 @@ TEST_F(TracezProcessor, RunningSnapshotThreadSafety) { */ TEST_F(TracezProcessor, SnapshotCompletedThreadSafety) { std::vector> spans; - StartManySpans(spans, tracer, 5); + StartManySpans(spans, tracer, 2000); - std::thread snapshots(GetManySnapshots, std::ref(processor), 5); + std::thread snapshots(GetManySnapshots, std::ref(processor), 2000); std::thread end(EndAllSpans, std::ref(spans)); snapshots.join(); @@ -573,10 +573,10 @@ TEST_F(TracezProcessor, SnapshotCompletedThreadSafety) { TEST_F(TracezProcessor, RunningSnapshotCompletedThreadSafety) { std::vector> spans1; std::vector> spans2; - StartManySpans(spans1, tracer, 5); + StartManySpans(spans1, tracer, 2000); - std::thread start(StartManySpans, std::ref(spans2), tracer, 5); - std::thread snapshots(GetManySnapshots, std::ref(processor), 5); + std::thread start(StartManySpans, std::ref(spans2), tracer, 2000); + std::thread snapshots(GetManySnapshots, std::ref(processor), 2000); std::thread end(EndAllSpans, std::ref(spans1)); start.join(); From 649367e424913d2aa9249ca5af66b4829b0e38b7 Mon Sep 17 00:00:00 2001 From: Janet Vu Date: Thu, 16 Jul 2020 08:27:20 -0400 Subject: [PATCH 19/19] Decrease spans created in thread safety tests --- ext/test/zpages/tracez_processor_test.cc | 36 ++++++++++++------------ 1 file changed, 18 insertions(+), 18 deletions(-) diff --git a/ext/test/zpages/tracez_processor_test.cc b/ext/test/zpages/tracez_processor_test.cc index 28fa4edc74..53fa1dba6b 100644 --- a/ext/test/zpages/tracez_processor_test.cc +++ b/ext/test/zpages/tracez_processor_test.cc @@ -475,8 +475,8 @@ TEST_F(TracezProcessor, RunningThreadSafety) { std::vector> spans1; std::vector> spans2; - std::thread start1(StartManySpans, std::ref(spans1), tracer, 2000); - std::thread start2(StartManySpans, std::ref(spans2), tracer, 2000); + std::thread start1(StartManySpans, std::ref(spans1), tracer, 500); + std::thread start2(StartManySpans, std::ref(spans2), tracer, 500); start1.join(); start2.join(); @@ -489,8 +489,8 @@ TEST_F(TracezProcessor, RunningThreadSafety) { TEST_F(TracezProcessor, CompletedThreadSafety) { std::vector> spans1; std::vector> spans2; - StartManySpans(spans1, tracer, 2000); - StartManySpans(spans2, tracer, 2000); + StartManySpans(spans1, tracer, 500); + StartManySpans(spans2, tracer, 500); std::thread end1(EndAllSpans, std::ref(spans1)); std::thread end2(EndAllSpans, std::ref(spans2)); @@ -506,16 +506,16 @@ TEST_F(TracezProcessor, CompletedThreadSafety) { TEST_F(TracezProcessor, SnapshotThreadSafety) { std::vector> spans; - std::thread snap1(GetManySnapshots, std::ref(processor), 2000); - std::thread snap2(GetManySnapshots, std::ref(processor), 2000); + std::thread snap1(GetManySnapshots, std::ref(processor), 500); + std::thread snap2(GetManySnapshots, std::ref(processor), 500); snap1.join(); snap2.join(); - StartManySpans(spans, tracer, 2000); + StartManySpans(spans, tracer, 500); - std::thread snap3(GetManySnapshots, std::ref(processor), 2000); - std::thread snap4(GetManySnapshots, std::ref(processor), 2000); + std::thread snap3(GetManySnapshots, std::ref(processor), 500); + std::thread snap4(GetManySnapshots, std::ref(processor), 500); snap3.join(); snap4.join(); @@ -528,9 +528,9 @@ TEST_F(TracezProcessor, SnapshotThreadSafety) { TEST_F(TracezProcessor, RunningCompletedThreadSafety) { std::vector> spans1; std::vector> spans2; - StartManySpans(spans1, tracer, 2000); + StartManySpans(spans1, tracer, 500); - std::thread start(StartManySpans, std::ref(spans2), tracer, 2000); + std::thread start(StartManySpans, std::ref(spans2), tracer, 500); std::thread end(EndAllSpans, std::ref(spans1)); start.join(); @@ -544,8 +544,8 @@ TEST_F(TracezProcessor, RunningCompletedThreadSafety) { TEST_F(TracezProcessor, RunningSnapshotThreadSafety) { std::vector> spans; - std::thread start(StartManySpans, std::ref(spans), tracer, 2000); - std::thread snapshots(GetManySnapshots, std::ref(processor), 2000); + std::thread start(StartManySpans, std::ref(spans), tracer, 500); + std::thread snapshots(GetManySnapshots, std::ref(processor), 500); start.join(); snapshots.join(); @@ -557,9 +557,9 @@ TEST_F(TracezProcessor, RunningSnapshotThreadSafety) { */ TEST_F(TracezProcessor, SnapshotCompletedThreadSafety) { std::vector> spans; - StartManySpans(spans, tracer, 2000); + StartManySpans(spans, tracer, 500); - std::thread snapshots(GetManySnapshots, std::ref(processor), 2000); + std::thread snapshots(GetManySnapshots, std::ref(processor), 500); std::thread end(EndAllSpans, std::ref(spans)); snapshots.join(); @@ -573,10 +573,10 @@ TEST_F(TracezProcessor, SnapshotCompletedThreadSafety) { TEST_F(TracezProcessor, RunningSnapshotCompletedThreadSafety) { std::vector> spans1; std::vector> spans2; - StartManySpans(spans1, tracer, 2000); + StartManySpans(spans1, tracer, 500); - std::thread start(StartManySpans, std::ref(spans2), tracer, 2000); - std::thread snapshots(GetManySnapshots, std::ref(processor), 2000); + std::thread start(StartManySpans, std::ref(spans2), tracer, 500); + std::thread snapshots(GetManySnapshots, std::ref(processor), 500); std::thread end(EndAllSpans, std::ref(spans1)); start.join();