From 1147049bc32578e43232a049bb77aa648c8aef22 Mon Sep 17 00:00:00 2001 From: Janet Vu Date: Mon, 6 Jul 2020 18:15:18 +0000 Subject: [PATCH 01/13] Consolidate tracez processor changes --- ext/BUILD | 7 + ext/CMakeLists.txt | 5 + .../ext/zpages/tracez_processor.h | 96 ++++ ext/src/CMakeLists.txt | 1 + ext/src/zpages/BUILD | 27 + ext/src/zpages/CMakeLists.txt | 1 + ext/src/zpages/tracez_processor.cc | 37 ++ ext/test/CMakeLists.txt | 1 + ext/test/zpages/BUILD | 11 + ext/test/zpages/CMakeLists.txt | 9 + ext/test/zpages/tracez_processor_test.cc | 528 ++++++++++++++++++ 11 files changed, 723 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..20ac53b54a --- /dev/null +++ b/ext/include/opentelemetry/ext/zpages/tracez_processor.h @@ -0,0 +1,96 @@ +#pragma once + +#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 an exporter and is + * used by the Data Aggregator to store spans before aggregating for TraceZ. + * + */ +class TracezSpanProcessor : public opentelemetry::sdk::trace::SpanProcessor { + public: + /** + * Initialize a span processor. + * @param exporter the exporter used by the span processor. Here, the purpose + * is still being discussed + */ + explicit TracezSpanProcessor(std::unique_ptr &&exporter) noexcept + : exporter_(std::move(exporter)) {} + + /** + * Create a span recordable using the associated exporter. + * @return a newly initialized recordable + */ + std::unique_ptr MakeRecordable() noexcept override + { + return exporter_->MakeRecordable(); + } + + /** + * 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 reference to running_spans + */ + std::unordered_set& GetRunningSpans() noexcept; + + /** + * Returns completed_spans, which is then cleared + */ + std::vector> GetCompletedSpans() noexcept; + + /** + * Export all ended spans that have not yet been exported. + * @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 + {} + + /** + * Shut down the processor and do any cleanup required. Ended spans are + * exported 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 + { + exporter_->Shutdown(timeout); + } + + private: + std::unique_ptr exporter_; + std::unordered_set running_spans_; + std::vector> completed_spans_; +}; +} // 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..0f07b73fc3 --- /dev/null +++ b/ext/src/zpages/CMakeLists.txt @@ -0,0 +1 @@ +add_library(opentelemetry_trace tracez_processer.cc) diff --git a/ext/src/zpages/tracez_processor.cc b/ext/src/zpages/tracez_processor.cc new file mode 100644 index 0000000000..b6f240d498 --- /dev/null +++ b/ext/src/zpages/tracez_processor.cc @@ -0,0 +1,37 @@ +#include "opentelemetry/ext/zpages/tracez_processor.h" + +OPENTELEMETRY_BEGIN_NAMESPACE +namespace ext { +namespace zpages { + + void TracezSpanProcessor::OnStart(opentelemetry::sdk::trace::Recordable &span) noexcept { + running_spans_.insert(&span); + } + + void TracezSpanProcessor::OnEnd(std::unique_ptr &&span) noexcept { + nostd::span> batch(&span, 1); + if (exporter_->Export(batch) == opentelemetry::sdk::trace::ExportResult::kFailure) { + std::cerr << "Error batching span\n"; + } + + auto completed_span = running_spans_.find(span.get()); + if (completed_span != running_spans_.end()) { + running_spans_.erase(completed_span); + completed_spans_.push_back(std::move(span)); + } + } + + std::unordered_set& TracezSpanProcessor::GetRunningSpans() noexcept { + return running_spans_; + } + + std::vector> TracezSpanProcessor::GetCompletedSpans() noexcept { + auto newly_completed_spans = std::move(completed_spans_); + completed_spans_.clear(); + return newly_completed_spans; + } + + +} // 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..14c11999ea --- /dev/null +++ b/ext/test/zpages/CMakeLists.txt @@ -0,0 +1,9 @@ +foreach(testname tracez_processer_test) + add_executable(${testname} "${testname}.cc") + target_link_libraries(${testname} ${GTEST_BOTH_LIBRARIES} + ${CMAKE_THREAD_LIBS_INIT} opentelemetry_trace) + gtest_add_tests( + TARGET ${testname} + TEST_PREFIX trace. + TEST_LIST ${testname}) +endforeach() diff --git a/ext/test/zpages/tracez_processor_test.cc b/ext/test/zpages/tracez_processor_test.cc new file mode 100644 index 0000000000..81ff79827f --- /dev/null +++ b/ext/test/zpages/tracez_processor_test.cc @@ -0,0 +1,528 @@ +#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; + +/** + * A mock exporter that switches a flag once a valid recordable was received. + */ +class MockSpanExporter final : public SpanExporter { + public: + MockSpanExporter(std::shared_ptr span_received, + std::shared_ptr shutdown_called) noexcept + : span_received_(span_received), shutdown_called_(shutdown_called) {} + + std::unique_ptr MakeRecordable() noexcept override { + return std::unique_ptr(new SpanData); + } + + ExportResult Export( + const opentelemetry::nostd::span> &spans) noexcept override { + for (auto &span : spans) { + if (span != nullptr) { + *span_received_ = true; + } + } + + return ExportResult::kSuccess; + } + + void Shutdown(std::chrono::microseconds timeout = std::chrono::microseconds(0)) noexcept override { + *shutdown_called_ = true; + } + + private: + std::shared_ptr span_received_; + std::shared_ptr shutdown_called_; +}; + + +/* + * Helper function uses the current processor to + * 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) { + + running = processor->GetRunningSpans(); + auto temp = processor->GetCompletedSpans(); + if (store_only_new_completed) { + completed.clear(); + completed = std::move(temp); + } + else { + std::move(temp.begin(), temp.end(), + std::inserter(completed, completed.end())); + } + temp.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 name + * 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(TracezSpanProcessor, GetSpansAndUseMockSpanExporter) { + std::shared_ptr span_received(new bool(false)); + std::shared_ptr shutdown_called(new bool(false)); + std::unique_ptr exporter(new MockSpanExporter(span_received, shutdown_called)); + std::shared_ptr processor(new TracezSpanProcessor(std::move(exporter))); + auto tracer = std::shared_ptr(new Tracer(processor)); + + auto span = processor.get()->MakeRecordable(); + + processor.get()->OnStart(*span); + ASSERT_FALSE(*span_received); + + auto running = processor->GetRunningSpans(); + auto completed = processor->GetCompletedSpans(); + + //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); + + processor.get()->OnEnd(std::move(span)); + ASSERT_TRUE(*span_received); + //span->End(); + UpdateSpans(processor, completed, running); + + //ASSERT_TRUE(ContainsNames(span_name, completed)); + ASSERT_EQ(running.size(), 0); + ASSERT_EQ(completed.size(), 1); + + processor.get()->Shutdown(); + ASSERT_TRUE(*shutdown_called); +} + +/* + * Test if the TraceZ processor correctly batches and + * exports spans + */ +TEST(TracezSpanProcessor, ToMockSpanExporter) { + std::shared_ptr span_received(new bool(false)); + std::shared_ptr shutdown_called(new bool(false)); + std::unique_ptr exporter(new MockSpanExporter(span_received, shutdown_called)); + TracezSpanProcessor processor(std::move(exporter)); + + auto recordable = processor.MakeRecordable(); + + processor.OnStart(*recordable); + ASSERT_FALSE(*span_received); + + processor.OnEnd(std::move(recordable)); + ASSERT_TRUE(*span_received); + + processor.Shutdown(); + ASSERT_TRUE(*shutdown_called); +} + + +/* + * Test if both span containers are empty when no spans + * exist or are added + */ +TEST(TracezSpanProcessor, NoSpans) { + std::shared_ptr span_received(new bool(false)); + std::shared_ptr shutdown_called(new bool(false)); + std::unique_ptr exporter(new MockSpanExporter(span_received, shutdown_called)); + std::shared_ptr processor(new TracezSpanProcessor(std::move(exporter))); + + ASSERT_EQ(processor->GetRunningSpans().size(), 0); + ASSERT_EQ(processor->GetCompletedSpans().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 span_received(new bool(false)); + std::shared_ptr shutdown_called(new bool(false)); + std::unique_ptr exporter(new MockSpanExporter(span_received, shutdown_called)); + std::shared_ptr processor(new TracezSpanProcessor(std::move(exporter))); + auto tracer = std::shared_ptr(new Tracer(processor)); + auto running = processor->GetRunningSpans(); + auto completed = processor->GetCompletedSpans(); + + 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 span_received(new bool(false)); + std::shared_ptr shutdown_called(new bool(false)); + std::unique_ptr exporter(new MockSpanExporter(span_received, shutdown_called)); + std::shared_ptr processor(new TracezSpanProcessor(std::move(exporter))); + auto tracer = std::shared_ptr(new Tracer(processor)); + auto running = processor->GetRunningSpans(); + auto completed = processor->GetCompletedSpans(); + + 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 span_received(new bool(false)); + std::shared_ptr shutdown_called(new bool(false)); + std::unique_ptr exporter(new MockSpanExporter(span_received, shutdown_called)); + std::shared_ptr processor(new TracezSpanProcessor(std::move(exporter))); + auto tracer = std::shared_ptr(new Tracer(processor)); + auto running = processor->GetRunningSpans(); + auto completed = processor->GetCompletedSpans(); + + std::vector span_names = {"s0", "s2", "s1", "s1", "s"}; + + // 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)); // s0 s2 s1 s1 s + ASSERT_EQ(running.size(), span_names.size()); + ASSERT_EQ(completed.size(), 0); + + // End 4th span + span_vars[3].get()->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].get()->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].get()->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].get()->End(); + span_vars[4].get()->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 span_received(new bool(false)); + std::shared_ptr shutdown_called(new bool(false)); + std::unique_ptr exporter(new MockSpanExporter(span_received, shutdown_called)); + std::shared_ptr processor(new TracezSpanProcessor(std::move(exporter))); + auto tracer = std::shared_ptr(new Tracer(processor)); + auto running = processor->GetRunningSpans(); + auto completed = processor->GetCompletedSpans(); + + std::vector span_names = {"s0", "s2", "s1", "s1", "s"}; + + // Start and store spans using span_names + std::vector> span_vars; + for (const auto &name : span_names) span_vars.push_back(tracer->StartSpan(name)); + + // End last span + span_vars[4].get()->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].get()->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].get()->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. + * Middle spans end first. Only new completed spans are stored. +*/ +TEST(TracezSpanProcessor, MultipleSpansRightContainerMiddleNewOnly) { + std::shared_ptr span_received(new bool(false)); + std::shared_ptr shutdown_called(new bool(false)); + std::unique_ptr exporter(new MockSpanExporter(span_received, shutdown_called)); + std::shared_ptr processor(new TracezSpanProcessor(std::move(exporter))); + auto tracer = std::shared_ptr(new Tracer(processor)); + auto running = processor->GetRunningSpans(); + auto completed = processor->GetCompletedSpans(); + + std::vector span_names = {"s0", "s2", "s1", "s1", "s"}; + + // 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, true)); // s0 s2 s1 s1 s + ASSERT_EQ(running.size(), span_names.size()); + ASSERT_EQ(completed.size(), 0); + + // End 4th span + span_vars[3].get()->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].get()->End(); + span_vars[2].get()->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].get()->End(); + span_vars[4].get()->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); +} + + +/* + * 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 span_received(new bool(false)); + std::shared_ptr shutdown_called(new bool(false)); + std::unique_ptr exporter(new MockSpanExporter(span_received, shutdown_called)); + std::shared_ptr processor(new TracezSpanProcessor(std::move(exporter))); + auto tracer = std::shared_ptr(new Tracer(processor)); + auto running = processor->GetRunningSpans(); + auto completed = processor->GetCompletedSpans(); + + std::vector span_names = {"s0", "s2", "s1", "s1", "s"}; + + // Start and store spans using span_names + std::vector> span_vars; + for (const auto &name : span_names) span_vars.push_back(tracer->StartSpan(name)); + + // End last span + span_vars[4].get()->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].get()->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].get()->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); +} + + From f721a1c0dd2595efae27f019e957b4a132c16b6d Mon Sep 17 00:00:00 2001 From: Janet Vu Date: Mon, 6 Jul 2020 22:44:45 +0000 Subject: [PATCH 02/13] Add some doc info --- ext/include/opentelemetry/ext/zpages/tracez_processor.h | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/ext/include/opentelemetry/ext/zpages/tracez_processor.h b/ext/include/opentelemetry/ext/zpages/tracez_processor.h index 20ac53b54a..c44e93dfa1 100644 --- a/ext/include/opentelemetry/ext/zpages/tracez_processor.h +++ b/ext/include/opentelemetry/ext/zpages/tracez_processor.h @@ -56,11 +56,15 @@ class TracezSpanProcessor : public opentelemetry::sdk::trace::SpanProcessor { /** * Returns a reference to running_spans + * @return currently running spans when the function is called */ std::unordered_set& GetRunningSpans() noexcept; /** - * Returns completed_spans, which is then cleared + * Returns completed_spans, which is then cleared to make room for new + * completed spans + * @return newly completed spans, which is defined by spans that completed + * after the last call of this function */ std::vector> GetCompletedSpans() noexcept; From 51b16e9e46262930fb4d0fbf0ed67bc6bed05fff Mon Sep 17 00:00:00 2001 From: Janet Vu Date: Tue, 7 Jul 2020 13:10:44 +0000 Subject: [PATCH 03/13] Add test helper function and cleaning --- ext/test/zpages/tracez_processor_test.cc | 181 ++++++++--------------- 1 file changed, 58 insertions(+), 123 deletions(-) diff --git a/ext/test/zpages/tracez_processor_test.cc b/ext/test/zpages/tracez_processor_test.cc index 81ff79827f..48dcc12483 100644 --- a/ext/test/zpages/tracez_processor_test.cc +++ b/ext/test/zpages/tracez_processor_test.cc @@ -42,12 +42,22 @@ class MockSpanExporter final : public SpanExporter { std::shared_ptr shutdown_called_; }; +/* + * Helper function to create a processor when the type of exporter doesn't + * matter + */ +std::shared_ptr MakeProcessor() { + std::shared_ptr span_received(new bool(false)); + std::shared_ptr shutdown_called(new bool(false)); + std::unique_ptr exporter(new MockSpanExporter(span_received, shutdown_called)); + std::shared_ptr processor(new TracezSpanProcessor(std::move(exporter))); + return processor; +} + /* - * Helper function uses the current processor to - * update spans contained in completed_spans and - * running_spans. completed acts contains all spans, - * unless marked otherwise + * 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, @@ -70,15 +80,12 @@ void UpdateSpans(std::shared_ptr& processor, /* - * 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. + * 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 + * 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, @@ -114,15 +121,12 @@ bool ContainsNames(const std::vector& names, /* - * 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 completed_spans + * 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 + * 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, @@ -155,47 +159,8 @@ bool ContainsNames(const std::vector& names, } - -TEST(TracezSpanProcessor, GetSpansAndUseMockSpanExporter) { - std::shared_ptr span_received(new bool(false)); - std::shared_ptr shutdown_called(new bool(false)); - std::unique_ptr exporter(new MockSpanExporter(span_received, shutdown_called)); - std::shared_ptr processor(new TracezSpanProcessor(std::move(exporter))); - auto tracer = std::shared_ptr(new Tracer(processor)); - - auto span = processor.get()->MakeRecordable(); - - processor.get()->OnStart(*span); - ASSERT_FALSE(*span_received); - - auto running = processor->GetRunningSpans(); - auto completed = processor->GetCompletedSpans(); - - //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); - - processor.get()->OnEnd(std::move(span)); - ASSERT_TRUE(*span_received); - //span->End(); - UpdateSpans(processor, completed, running); - - //ASSERT_TRUE(ContainsNames(span_name, completed)); - ASSERT_EQ(running.size(), 0); - ASSERT_EQ(completed.size(), 1); - - processor.get()->Shutdown(); - ASSERT_TRUE(*shutdown_called); -} - /* - * Test if the TraceZ processor correctly batches and - * exports spans + * Test if the TraceZ processor correctly batches and exports spans */ TEST(TracezSpanProcessor, ToMockSpanExporter) { std::shared_ptr span_received(new bool(false)); @@ -217,14 +182,10 @@ TEST(TracezSpanProcessor, ToMockSpanExporter) { /* - * 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 */ TEST(TracezSpanProcessor, NoSpans) { - std::shared_ptr span_received(new bool(false)); - std::shared_ptr shutdown_called(new bool(false)); - std::unique_ptr exporter(new MockSpanExporter(span_received, shutdown_called)); - std::shared_ptr processor(new TracezSpanProcessor(std::move(exporter))); + auto processor = MakeProcessor(); ASSERT_EQ(processor->GetRunningSpans().size(), 0); ASSERT_EQ(processor->GetCompletedSpans().size(), 0); @@ -233,14 +194,11 @@ TEST(TracezSpanProcessor, NoSpans) { /* - * Test if a single span moves from running to completed - * at expected times. All completed spans are stored. + * Test if a single span moves from running to completed at expected times. + * All completed spans are stored. */ TEST(TracezSpanProcessor, OneSpanRightContainerStored) { - std::shared_ptr span_received(new bool(false)); - std::shared_ptr shutdown_called(new bool(false)); - std::unique_ptr exporter(new MockSpanExporter(span_received, shutdown_called)); - std::shared_ptr processor(new TracezSpanProcessor(std::move(exporter))); + auto processor = MakeProcessor(); auto tracer = std::shared_ptr(new Tracer(processor)); auto running = processor->GetRunningSpans(); auto completed = processor->GetCompletedSpans(); @@ -265,15 +223,12 @@ 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 completed spans are stored. + * 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 span_received(new bool(false)); - std::shared_ptr shutdown_called(new bool(false)); - std::unique_ptr exporter(new MockSpanExporter(span_received, shutdown_called)); - std::shared_ptr processor(new TracezSpanProcessor(std::move(exporter))); + auto processor = MakeProcessor(); auto tracer = std::shared_ptr(new Tracer(processor)); auto running = processor->GetRunningSpans(); auto completed = processor->GetCompletedSpans(); @@ -304,22 +259,17 @@ 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. + * 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 span_received(new bool(false)); - std::shared_ptr shutdown_called(new bool(false)); - std::unique_ptr exporter(new MockSpanExporter(span_received, shutdown_called)); - std::shared_ptr processor(new TracezSpanProcessor(std::move(exporter))); + auto processor = MakeProcessor(); auto tracer = std::shared_ptr(new Tracer(processor)); auto running = processor->GetRunningSpans(); auto completed = processor->GetCompletedSpans(); std::vector span_names = {"s0", "s2", "s1", "s1", "s"}; - // 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); @@ -329,7 +279,7 @@ TEST(TracezSpanProcessor, MultipleSpansRightContainerMiddleStored) { ASSERT_EQ(completed.size(), 0); // End 4th span - span_vars[3].get()->End(); + span_vars[3]->End(); UpdateSpans(processor, completed, running); ASSERT_TRUE(ContainsNames(span_names, running, 0, 3)); // s0 s2 s1 @@ -339,7 +289,7 @@ TEST(TracezSpanProcessor, MultipleSpansRightContainerMiddleStored) { ASSERT_EQ(completed.size(), 1); // End 2nd span - span_vars[1].get()->End(); + span_vars[1]->End(); UpdateSpans(processor, completed, running); ASSERT_TRUE(ContainsNames(span_names, running, 0, 1)); // s0 @@ -351,7 +301,7 @@ TEST(TracezSpanProcessor, MultipleSpansRightContainerMiddleStored) { ASSERT_EQ(completed.size(), 2); // End 3rd span (last middle span) - span_vars[2].get()->End(); + span_vars[2]->End(); UpdateSpans(processor, completed, running); ASSERT_TRUE(ContainsNames(span_names, running, 0, 1)); // s0 @@ -361,8 +311,8 @@ TEST(TracezSpanProcessor, MultipleSpansRightContainerMiddleStored) { ASSERT_EQ(completed.size(), 3); // End remaining Spans - span_vars[0].get()->End(); - span_vars[4].get()->End(); + span_vars[0]->End(); + span_vars[4]->End(); UpdateSpans(processor, completed, running); ASSERT_TRUE(ContainsNames(span_names, completed)); // s0 s2 s1 s1 s @@ -372,27 +322,22 @@ 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. + * 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 span_received(new bool(false)); - std::shared_ptr shutdown_called(new bool(false)); - std::unique_ptr exporter(new MockSpanExporter(span_received, shutdown_called)); - std::shared_ptr processor(new TracezSpanProcessor(std::move(exporter))); + auto processor = MakeProcessor(); auto tracer = std::shared_ptr(new Tracer(processor)); auto running = processor->GetRunningSpans(); auto completed = processor->GetCompletedSpans(); std::vector span_names = {"s0", "s2", "s1", "s1", "s"}; - // Start and store spans using span_names std::vector> span_vars; for (const auto &name : span_names) span_vars.push_back(tracer->StartSpan(name)); // End last span - span_vars[4].get()->End(); + span_vars[4]->End(); UpdateSpans(processor, completed, running); ASSERT_TRUE(ContainsNames(span_names, running, 0, 4)); // s0 s2 s1 s1 @@ -401,7 +346,7 @@ TEST(TracezSpanProcessor, MultipleSpansRightContainerOuterStored) { ASSERT_EQ(completed.size(), 1); // End first span - span_vars[0].get()->End(); + span_vars[0]->End(); UpdateSpans(processor, completed, running); ASSERT_TRUE(ContainsNames(span_names, running, 1, 4)); // s2 s1 s1 @@ -411,7 +356,7 @@ TEST(TracezSpanProcessor, MultipleSpansRightContainerOuterStored) { ASSERT_EQ(completed.size(), 2); // End remaining Spans - for (int i = 1; i < 4; i++) span_vars[i].get()->End(); + 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 @@ -421,22 +366,17 @@ TEST(TracezSpanProcessor, MultipleSpansRightContainerOuterStored) { /* - * 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 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 span_received(new bool(false)); - std::shared_ptr shutdown_called(new bool(false)); - std::unique_ptr exporter(new MockSpanExporter(span_received, shutdown_called)); - std::shared_ptr processor(new TracezSpanProcessor(std::move(exporter))); + auto processor = MakeProcessor(); auto tracer = std::shared_ptr(new Tracer(processor)); auto running = processor->GetRunningSpans(); auto completed = processor->GetCompletedSpans(); std::vector span_names = {"s0", "s2", "s1", "s1", "s"}; - // 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); @@ -446,7 +386,7 @@ TEST(TracezSpanProcessor, MultipleSpansRightContainerMiddleNewOnly) { ASSERT_EQ(completed.size(), 0); // End 4th span - span_vars[3].get()->End(); + span_vars[3]->End(); UpdateSpans(processor, completed, running, true); ASSERT_TRUE(ContainsNames(span_names, running, 0, 3)); // s0 s2 s1 @@ -456,8 +396,8 @@ TEST(TracezSpanProcessor, MultipleSpansRightContainerMiddleNewOnly) { ASSERT_EQ(completed.size(), 1); // End 2nd and 3rd span - span_vars[1].get()->End(); - span_vars[2].get()->End(); + span_vars[1]->End(); + span_vars[2]->End(); UpdateSpans(processor, completed, running, true); ASSERT_TRUE(ContainsNames(span_names, running, 0, 1)); // s0 @@ -467,8 +407,8 @@ TEST(TracezSpanProcessor, MultipleSpansRightContainerMiddleNewOnly) { ASSERT_EQ(completed.size(), 2); // End remaining Spans - span_vars[0].get()->End(); - span_vars[4].get()->End(); + span_vars[0]->End(); + span_vars[4]->End(); UpdateSpans(processor, completed, running, true); ASSERT_TRUE(ContainsNames(span_names, completed, 0, 1)); // s0 @@ -479,27 +419,22 @@ 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. Only new completed spans are stored. */ TEST(TracezSpanProcessor, MultipleSpansRightContainerOuterNewOnly) { - std::shared_ptr span_received(new bool(false)); - std::shared_ptr shutdown_called(new bool(false)); - std::unique_ptr exporter(new MockSpanExporter(span_received, shutdown_called)); - std::shared_ptr processor(new TracezSpanProcessor(std::move(exporter))); + auto processor = MakeProcessor(); auto tracer = std::shared_ptr(new Tracer(processor)); auto running = processor->GetRunningSpans(); auto completed = processor->GetCompletedSpans(); std::vector span_names = {"s0", "s2", "s1", "s1", "s"}; - // Start and store spans using span_names std::vector> span_vars; for (const auto &name : span_names) span_vars.push_back(tracer->StartSpan(name)); // End last span - span_vars[4].get()->End(); + span_vars[4]->End(); UpdateSpans(processor, completed, running, true); ASSERT_TRUE(ContainsNames(span_names, running, 0, 4, true)); // s0 s2 s1 s1 @@ -508,7 +443,7 @@ TEST(TracezSpanProcessor, MultipleSpansRightContainerOuterNewOnly) { ASSERT_EQ(completed.size(), 1); // End first span - span_vars[0].get()->End(); + span_vars[0]->End(); UpdateSpans(processor, completed, running, true); ASSERT_TRUE(ContainsNames(span_names, running, 1, 4, true)); // s2 s1 s1 @@ -517,7 +452,7 @@ TEST(TracezSpanProcessor, MultipleSpansRightContainerOuterNewOnly) { ASSERT_EQ(completed.size(), 1); // End remaining middle pans - for (int i = 1; i < 4; i++) span_vars[i].get()->End(); + 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 From b6719124765282560a9e4efe225d27ed9a70aa07 Mon Sep 17 00:00:00 2001 From: Janet Vu Date: Tue, 7 Jul 2020 13:17:41 +0000 Subject: [PATCH 04/13] Ensure completed is empty if no new spans --- ext/test/zpages/tracez_processor_test.cc | 45 ++++++++++++++++++++++++ 1 file changed, 45 insertions(+) diff --git a/ext/test/zpages/tracez_processor_test.cc b/ext/test/zpages/tracez_processor_test.cc index 48dcc12483..de478e9cc5 100644 --- a/ext/test/zpages/tracez_processor_test.cc +++ b/ext/test/zpages/tracez_processor_test.cc @@ -364,6 +364,39 @@ TEST(TracezSpanProcessor, MultipleSpansRightContainerOuterStored) { 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) { + auto processor = MakeProcessor(); + auto tracer = std::shared_ptr(new Tracer(processor)); + auto running = processor->GetRunningSpans(); + auto completed = processor->GetCompletedSpans(); + + 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, @@ -415,6 +448,12 @@ TEST(TracezSpanProcessor, MultipleSpansRightContainerMiddleNewOnly) { 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); } @@ -458,6 +497,12 @@ TEST(TracezSpanProcessor, MultipleSpansRightContainerOuterNewOnly) { 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 d69a2b34c86c58d17b915885bf836d3230fbb541 Mon Sep 17 00:00:00 2001 From: Janet Vu Date: Tue, 7 Jul 2020 15:42:32 +0000 Subject: [PATCH 05/13] Add nullptr check --- ext/src/zpages/tracez_processor.cc | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/ext/src/zpages/tracez_processor.cc b/ext/src/zpages/tracez_processor.cc index b6f240d498..53b668a4a1 100644 --- a/ext/src/zpages/tracez_processor.cc +++ b/ext/src/zpages/tracez_processor.cc @@ -13,11 +13,12 @@ namespace zpages { if (exporter_->Export(batch) == opentelemetry::sdk::trace::ExportResult::kFailure) { std::cerr << "Error batching span\n"; } - - auto completed_span = running_spans_.find(span.get()); - if (completed_span != running_spans_.end()) { - running_spans_.erase(completed_span); - completed_spans_.push_back(std::move(span)); + else if (span != nullptr) { + auto completed_span = running_spans_.find(span.get()); + if (completed_span != running_spans_.end()) { + running_spans_.erase(completed_span); + completed_spans_.push_back(std::move(span)); + } } } From f32cf66507a50d3755593784cfc260630597adb8 Mon Sep 17 00:00:00 2001 From: Janet Vu Date: Tue, 7 Jul 2020 18:36:28 +0000 Subject: [PATCH 06/13] Remove exporter, add span struct, shutdown signal --- .../ext/zpages/tracez_processor.h | 40 +++---- ext/src/zpages/tracez_processor.cc | 32 +++-- ext/test/zpages/tracez_processor_test.cc | 110 +++++------------- 3 files changed, 65 insertions(+), 117 deletions(-) diff --git a/ext/include/opentelemetry/ext/zpages/tracez_processor.h b/ext/include/opentelemetry/ext/zpages/tracez_processor.h index c44e93dfa1..ca24a7daae 100644 --- a/ext/include/opentelemetry/ext/zpages/tracez_processor.h +++ b/ext/include/opentelemetry/ext/zpages/tracez_processor.h @@ -24,13 +24,18 @@ namespace zpages */ class TracezSpanProcessor : public opentelemetry::sdk::trace::SpanProcessor { public: + + struct CollectedSpans { + std::unordered_set running; + std::vector> completed; + }; + /** * Initialize a span processor. * @param exporter the exporter used by the span processor. Here, the purpose * is still being discussed */ - explicit TracezSpanProcessor(std::unique_ptr &&exporter) noexcept - : exporter_(std::move(exporter)) {} + explicit TracezSpanProcessor() noexcept {} /** * Create a span recordable using the associated exporter. @@ -38,7 +43,7 @@ class TracezSpanProcessor : public opentelemetry::sdk::trace::SpanProcessor { */ std::unique_ptr MakeRecordable() noexcept override { - return exporter_->MakeRecordable(); + return std::unique_ptr(new opentelemetry::sdk::trace::SpanData); } /** @@ -55,18 +60,13 @@ class TracezSpanProcessor : public opentelemetry::sdk::trace::SpanProcessor { void OnEnd(std::unique_ptr &&span) noexcept override; /** - * Returns a reference to running_spans - * @return currently running spans when the function is called - */ - std::unordered_set& GetRunningSpans() noexcept; - - /** - * Returns completed_spans, which is then cleared to make room for new - * completed spans - * @return newly completed spans, which is defined by spans that completed - * after the last call of this function + * 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 + * @return snapshot of all currently running spans and newly completed spans + * at the time that the function is called */ - std::vector> GetCompletedSpans() noexcept; + CollectedSpans GetSpanSnapshot() noexcept; /** * Export all ended spans that have not yet been exported. @@ -75,8 +75,9 @@ class TracezSpanProcessor : public opentelemetry::sdk::trace::SpanProcessor { */ void ForceFlush( std::chrono::microseconds timeout = std::chrono::microseconds(0)) noexcept override - {} - + { + if (shutdown_signal_received_) return; + } /** * Shut down the processor and do any cleanup required. Ended spans are * exported before shutdown. After the call to Shutdown, subsequent calls to @@ -87,13 +88,12 @@ class TracezSpanProcessor : public opentelemetry::sdk::trace::SpanProcessor { */ void Shutdown(std::chrono::microseconds timeout = std::chrono::microseconds(0)) noexcept override { - exporter_->Shutdown(timeout); + shutdown_signal_received_ = true; } private: - std::unique_ptr exporter_; - std::unordered_set running_spans_; - std::vector> completed_spans_; + 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 53b668a4a1..5613f21108 100644 --- a/ext/src/zpages/tracez_processor.cc +++ b/ext/src/zpages/tracez_processor.cc @@ -5,31 +5,27 @@ namespace ext { namespace zpages { void TracezSpanProcessor::OnStart(opentelemetry::sdk::trace::Recordable &span) noexcept { - running_spans_.insert(&span); + if (shutdown_signal_received_) return; + spans_.running.insert(&span); } void TracezSpanProcessor::OnEnd(std::unique_ptr &&span) noexcept { - nostd::span> batch(&span, 1); - if (exporter_->Export(batch) == opentelemetry::sdk::trace::ExportResult::kFailure) { - std::cerr << "Error batching span\n"; - } - else if (span != nullptr) { - auto completed_span = running_spans_.find(span.get()); - if (completed_span != running_spans_.end()) { - running_spans_.erase(completed_span); - completed_spans_.push_back(std::move(span)); - } + 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)); } } - std::unordered_set& TracezSpanProcessor::GetRunningSpans() noexcept { - return running_spans_; - } - std::vector> TracezSpanProcessor::GetCompletedSpans() noexcept { - auto newly_completed_spans = std::move(completed_spans_); - completed_spans_.clear(); - return newly_completed_spans; + 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; } diff --git a/ext/test/zpages/tracez_processor_test.cc b/ext/test/zpages/tracez_processor_test.cc index de478e9cc5..ce8cd7ba75 100644 --- a/ext/test/zpages/tracez_processor_test.cc +++ b/ext/test/zpages/tracez_processor_test.cc @@ -9,48 +9,13 @@ using namespace opentelemetry::sdk::trace; using namespace opentelemetry::ext::zpages; -/** - * A mock exporter that switches a flag once a valid recordable was received. - */ -class MockSpanExporter final : public SpanExporter { - public: - MockSpanExporter(std::shared_ptr span_received, - std::shared_ptr shutdown_called) noexcept - : span_received_(span_received), shutdown_called_(shutdown_called) {} - - std::unique_ptr MakeRecordable() noexcept override { - return std::unique_ptr(new SpanData); - } - - ExportResult Export( - const opentelemetry::nostd::span> &spans) noexcept override { - for (auto &span : spans) { - if (span != nullptr) { - *span_received_ = true; - } - } - - return ExportResult::kSuccess; - } - - void Shutdown(std::chrono::microseconds timeout = std::chrono::microseconds(0)) noexcept override { - *shutdown_called_ = true; - } - - private: - std::shared_ptr span_received_; - std::shared_ptr shutdown_called_; -}; /* * Helper function to create a processor when the type of exporter doesn't * matter */ std::shared_ptr MakeProcessor() { - std::shared_ptr span_received(new bool(false)); - std::shared_ptr shutdown_called(new bool(false)); - std::unique_ptr exporter(new MockSpanExporter(span_received, shutdown_called)); - std::shared_ptr processor(new TracezSpanProcessor(std::move(exporter))); + std::shared_ptr processor(new TracezSpanProcessor()); return processor; } @@ -63,18 +28,17 @@ void UpdateSpans(std::shared_ptr& processor, std::vector>& completed, std::unordered_set& running, bool store_only_new_completed = false) { - - running = processor->GetRunningSpans(); - auto temp = processor->GetCompletedSpans(); + auto spans = processor->GetSpanSnapshot(); + running = spans.running; if (store_only_new_completed) { completed.clear(); - completed = std::move(temp); + completed = std::move(spans.completed); } else { - std::move(temp.begin(), temp.end(), + std::move(spans.completed.begin(), spans.completed.end(), std::inserter(completed, completed.end())); } - temp.clear(); + spans.completed.clear(); } @@ -159,36 +123,17 @@ bool ContainsNames(const std::vector& names, } -/* - * Test if the TraceZ processor correctly batches and exports spans - */ -TEST(TracezSpanProcessor, ToMockSpanExporter) { - std::shared_ptr span_received(new bool(false)); - std::shared_ptr shutdown_called(new bool(false)); - std::unique_ptr exporter(new MockSpanExporter(span_received, shutdown_called)); - TracezSpanProcessor processor(std::move(exporter)); - - auto recordable = processor.MakeRecordable(); - - processor.OnStart(*recordable); - ASSERT_FALSE(*span_received); - - processor.OnEnd(std::move(recordable)); - ASSERT_TRUE(*span_received); - - processor.Shutdown(); - ASSERT_TRUE(*shutdown_called); -} - /* * Test if both span containers are empty when no spans exist or are added */ TEST(TracezSpanProcessor, NoSpans) { auto processor = MakeProcessor(); + auto spans = processor->GetSpanSnapshot(); + auto recordable = processor->MakeRecordable(); - ASSERT_EQ(processor->GetRunningSpans().size(), 0); - ASSERT_EQ(processor->GetCompletedSpans().size(), 0); + ASSERT_EQ(spans.running.size(), 0); + ASSERT_EQ(spans.running.size(), 0); } @@ -200,8 +145,9 @@ TEST(TracezSpanProcessor, NoSpans) { TEST(TracezSpanProcessor, OneSpanRightContainerStored) { auto processor = MakeProcessor(); auto tracer = std::shared_ptr(new Tracer(processor)); - auto running = processor->GetRunningSpans(); - auto completed = processor->GetCompletedSpans(); + auto spans = processor->GetSpanSnapshot(); + auto running = spans.running; + auto completed = std::move(spans.completed); std::vector span_name = { "span" }; @@ -230,8 +176,9 @@ TEST(TracezSpanProcessor, OneSpanRightContainerStored) { TEST(TracezSpanProcessor, MultipleSpansRightContainerStored) { auto processor = MakeProcessor(); auto tracer = std::shared_ptr(new Tracer(processor)); - auto running = processor->GetRunningSpans(); - auto completed = processor->GetCompletedSpans(); + auto spans = processor->GetSpanSnapshot(); + auto running = spans.running; + auto completed = std::move(spans.completed); ASSERT_EQ(running.size(), 0); ASSERT_EQ(completed.size(), 0); @@ -265,8 +212,9 @@ TEST(TracezSpanProcessor, MultipleSpansRightContainerStored) { TEST(TracezSpanProcessor, MultipleSpansRightContainerMiddleStored) { auto processor = MakeProcessor(); auto tracer = std::shared_ptr(new Tracer(processor)); - auto running = processor->GetRunningSpans(); - auto completed = processor->GetCompletedSpans(); + auto spans = processor->GetSpanSnapshot(); + auto running = spans.running; + auto completed = std::move(spans.completed); std::vector span_names = {"s0", "s2", "s1", "s1", "s"}; @@ -328,8 +276,9 @@ TEST(TracezSpanProcessor, MultipleSpansRightContainerMiddleStored) { TEST(TracezSpanProcessor, MultipleSpansRightContainerOuterStored) { auto processor = MakeProcessor(); auto tracer = std::shared_ptr(new Tracer(processor)); - auto running = processor->GetRunningSpans(); - auto completed = processor->GetCompletedSpans(); + auto spans = processor->GetSpanSnapshot(); + auto running = spans.running; + auto completed = std::move(spans.completed); std::vector span_names = {"s0", "s2", "s1", "s1", "s"}; @@ -371,8 +320,9 @@ TEST(TracezSpanProcessor, MultipleSpansRightContainerOuterStored) { TEST(TracezSpanProcessor, OneSpanRightContainerNewOnly) { auto processor = MakeProcessor(); auto tracer = std::shared_ptr(new Tracer(processor)); - auto running = processor->GetRunningSpans(); - auto completed = processor->GetCompletedSpans(); + auto spans = processor->GetSpanSnapshot(); + auto running = spans.running; + auto completed = std::move(spans.completed); std::vector span_name = { "span" }; @@ -405,8 +355,9 @@ TEST(TracezSpanProcessor, OneSpanRightContainerNewOnly) { TEST(TracezSpanProcessor, MultipleSpansRightContainerMiddleNewOnly) { auto processor = MakeProcessor(); auto tracer = std::shared_ptr(new Tracer(processor)); - auto running = processor->GetRunningSpans(); - auto completed = processor->GetCompletedSpans(); + auto spans = processor->GetSpanSnapshot(); + auto running = spans.running; + auto completed = std::move(spans.completed); std::vector span_names = {"s0", "s2", "s1", "s1", "s"}; @@ -464,8 +415,9 @@ TEST(TracezSpanProcessor, MultipleSpansRightContainerMiddleNewOnly) { TEST(TracezSpanProcessor, MultipleSpansRightContainerOuterNewOnly) { auto processor = MakeProcessor(); auto tracer = std::shared_ptr(new Tracer(processor)); - auto running = processor->GetRunningSpans(); - auto completed = processor->GetCompletedSpans(); + auto spans = processor->GetSpanSnapshot(); + auto running = spans.running; + auto completed = std::move(spans.completed); std::vector span_names = {"s0", "s2", "s1", "s1", "s"}; From 3f09ffb4dbb5daf6435f9be5c97aa051872db342 Mon Sep 17 00:00:00 2001 From: Janet Vu Date: Tue, 7 Jul 2020 18:38:51 +0000 Subject: [PATCH 07/13] Add todo notes --- ext/include/opentelemetry/ext/zpages/tracez_processor.h | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/ext/include/opentelemetry/ext/zpages/tracez_processor.h b/ext/include/opentelemetry/ext/zpages/tracez_processor.h index ca24a7daae..307b64cbac 100644 --- a/ext/include/opentelemetry/ext/zpages/tracez_processor.h +++ b/ext/include/opentelemetry/ext/zpages/tracez_processor.h @@ -77,6 +77,7 @@ class TracezSpanProcessor : public opentelemetry::sdk::trace::SpanProcessor { std::chrono::microseconds timeout = std::chrono::microseconds(0)) noexcept override { if (shutdown_signal_received_) return; + // TODO: figure out how to export to data aggregator when this is called? } /** * Shut down the processor and do any cleanup required. Ended spans are @@ -88,7 +89,7 @@ class TracezSpanProcessor : public opentelemetry::sdk::trace::SpanProcessor { */ void Shutdown(std::chrono::microseconds timeout = std::chrono::microseconds(0)) noexcept override { - shutdown_signal_received_ = true; + shutdown_signal_received_ = true; // TODO: what cleanup do we need? } private: From 1a0875f111a349e7a07860964b7a05ab0378cdfc Mon Sep 17 00:00:00 2001 From: Janet Vu Date: Tue, 7 Jul 2020 18:40:27 +0000 Subject: [PATCH 08/13] Add space for formatting --- ext/src/zpages/tracez_processor.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ext/src/zpages/tracez_processor.cc b/ext/src/zpages/tracez_processor.cc index 5613f21108..28bec9250c 100644 --- a/ext/src/zpages/tracez_processor.cc +++ b/ext/src/zpages/tracez_processor.cc @@ -10,7 +10,7 @@ namespace zpages { } void TracezSpanProcessor::OnEnd(std::unique_ptr &&span) noexcept { - if (shutdown_signal_received_ || span == nullptr) return; + 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); From af4a6d8837ebb0b8aa11a7e4c39004a40f0282eb Mon Sep 17 00:00:00 2001 From: Janet Vu Date: Wed, 8 Jul 2020 14:44:50 +0000 Subject: [PATCH 09/13] Update comments/shutdown/flush, add TODOs --- .../ext/zpages/tracez_processor.h | 21 +++++++++++-------- ext/test/zpages/tracez_processor_test.cc | 8 +++++-- 2 files changed, 18 insertions(+), 11 deletions(-) diff --git a/ext/include/opentelemetry/ext/zpages/tracez_processor.h b/ext/include/opentelemetry/ext/zpages/tracez_processor.h index 307b64cbac..fb966f7ebd 100644 --- a/ext/include/opentelemetry/ext/zpages/tracez_processor.h +++ b/ext/include/opentelemetry/ext/zpages/tracez_processor.h @@ -18,8 +18,8 @@ namespace ext namespace zpages { /** - * The span processor passes finished recordables to an exporter and is - * used by the Data Aggregator to store spans before aggregating for TraceZ. + * 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 { @@ -32,13 +32,11 @@ class TracezSpanProcessor : public opentelemetry::sdk::trace::SpanProcessor { /** * Initialize a span processor. - * @param exporter the exporter used by the span processor. Here, the purpose - * is still being discussed */ explicit TracezSpanProcessor() noexcept {} /** - * Create a span recordable using the associated exporter. + * Create a span recordable * @return a newly initialized recordable */ std::unique_ptr MakeRecordable() noexcept override @@ -62,14 +60,16 @@ class TracezSpanProcessor : public opentelemetry::sdk::trace::SpanProcessor { /** * 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 + * 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; /** - * Export all ended spans that have not yet been exported. + * 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. */ @@ -77,11 +77,13 @@ class TracezSpanProcessor : public opentelemetry::sdk::trace::SpanProcessor { std::chrono::microseconds timeout = std::chrono::microseconds(0)) noexcept override { if (shutdown_signal_received_) return; - // TODO: figure out how to export to data aggregator when this is called? + // TODO: figure out how to send spans to data aggregator when this is called? + // should running spans be forced to end, and their status reflect this + // accordingly? } /** * Shut down the processor and do any cleanup required. Ended spans are - * exported before shutdown. After the call to Shutdown, subsequent calls to + * 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 @@ -89,6 +91,7 @@ class TracezSpanProcessor : public opentelemetry::sdk::trace::SpanProcessor { */ void Shutdown(std::chrono::microseconds timeout = std::chrono::microseconds(0)) noexcept override { + ForceFlush(); shutdown_signal_received_ = true; // TODO: what cleanup do we need? } diff --git a/ext/test/zpages/tracez_processor_test.cc b/ext/test/zpages/tracez_processor_test.cc index ce8cd7ba75..05c95f7c86 100644 --- a/ext/test/zpages/tracez_processor_test.cc +++ b/ext/test/zpages/tracez_processor_test.cc @@ -10,9 +10,13 @@ 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 when the type of exporter doesn't - * matter + * 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()); From 9e556f8ad2773a8a126ba1ce25203a5ab841a30b Mon Sep 17 00:00:00 2001 From: Janet Vu Date: Wed, 8 Jul 2020 14:49:22 +0000 Subject: [PATCH 10/13] Use timeout for shutdown and flush --- ext/include/opentelemetry/ext/zpages/tracez_processor.h | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/ext/include/opentelemetry/ext/zpages/tracez_processor.h b/ext/include/opentelemetry/ext/zpages/tracez_processor.h index fb966f7ebd..d7ae3c87f9 100644 --- a/ext/include/opentelemetry/ext/zpages/tracez_processor.h +++ b/ext/include/opentelemetry/ext/zpages/tracez_processor.h @@ -5,6 +5,7 @@ #include #include #include +#include #include #include "opentelemetry/sdk/trace/recordable.h" @@ -76,6 +77,7 @@ class TracezSpanProcessor : public opentelemetry::sdk::trace::SpanProcessor { void ForceFlush( std::chrono::microseconds timeout = std::chrono::microseconds(0)) noexcept override { + std::this_thread::sleep_for(timeout); if (shutdown_signal_received_) return; // TODO: figure out how to send spans to data aggregator when this is called? // should running spans be forced to end, and their status reflect this @@ -91,7 +93,7 @@ class TracezSpanProcessor : public opentelemetry::sdk::trace::SpanProcessor { */ void Shutdown(std::chrono::microseconds timeout = std::chrono::microseconds(0)) noexcept override { - ForceFlush(); + ForceFlush(timeout); shutdown_signal_received_ = true; // TODO: what cleanup do we need? } From eb81ff6940ba261c63ea8f041df1008f72f8af58 Mon Sep 17 00:00:00 2001 From: Janet Vu Date: Wed, 8 Jul 2020 14:54:11 +0000 Subject: [PATCH 11/13] Move timeout line --- ext/include/opentelemetry/ext/zpages/tracez_processor.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ext/include/opentelemetry/ext/zpages/tracez_processor.h b/ext/include/opentelemetry/ext/zpages/tracez_processor.h index d7ae3c87f9..c9125573af 100644 --- a/ext/include/opentelemetry/ext/zpages/tracez_processor.h +++ b/ext/include/opentelemetry/ext/zpages/tracez_processor.h @@ -77,8 +77,8 @@ class TracezSpanProcessor : public opentelemetry::sdk::trace::SpanProcessor { void ForceFlush( std::chrono::microseconds timeout = std::chrono::microseconds(0)) noexcept override { - std::this_thread::sleep_for(timeout); if (shutdown_signal_received_) return; + std::this_thread::sleep_for(timeout); // TODO: figure out how to send spans to data aggregator when this is called? // should running spans be forced to end, and their status reflect this // accordingly? From 2d061af5d4f579cade0dbd0eac1a83c51a2919c4 Mon Sep 17 00:00:00 2001 From: Janet Vu Date: Wed, 8 Jul 2020 15:36:45 +0000 Subject: [PATCH 12/13] Revert "Move timeout line" This reverts commit eb81ff6940ba261c63ea8f041df1008f72f8af58. --- ext/include/opentelemetry/ext/zpages/tracez_processor.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ext/include/opentelemetry/ext/zpages/tracez_processor.h b/ext/include/opentelemetry/ext/zpages/tracez_processor.h index c9125573af..d7ae3c87f9 100644 --- a/ext/include/opentelemetry/ext/zpages/tracez_processor.h +++ b/ext/include/opentelemetry/ext/zpages/tracez_processor.h @@ -77,8 +77,8 @@ class TracezSpanProcessor : public opentelemetry::sdk::trace::SpanProcessor { void ForceFlush( std::chrono::microseconds timeout = std::chrono::microseconds(0)) noexcept override { - if (shutdown_signal_received_) return; std::this_thread::sleep_for(timeout); + if (shutdown_signal_received_) return; // TODO: figure out how to send spans to data aggregator when this is called? // should running spans be forced to end, and their status reflect this // accordingly? From aa1b39c1b20108b5850e06c1a9d948a4b8d99a0b Mon Sep 17 00:00:00 2001 From: Janet Vu Date: Wed, 8 Jul 2020 15:44:17 +0000 Subject: [PATCH 13/13] Revert "Merge branch 'zpages-tracez-processor1' of https://github.com/kmanghat/opentelemetry-cpp into zpages-tracez-processor1" This reverts commit 38cc77aec4073b837d926e8423a296fb8ddc1d76, reversing changes made to 2d061af5d4f579cade0dbd0eac1a83c51a2919c4. --- exporters/otlp/BUILD | 9 - exporters/otlp/CMakeLists.txt | 8 - exporters/otlp/recordable.cc | 29 --- exporters/otlp/recordable.h | 12 +- exporters/otlp/recordable_test.cc | 73 ------ sdk/include/opentelemetry/sdk/trace/tracer.h | 11 +- .../opentelemetry/sdk/trace/tracer_provider.h | 16 +- sdk/src/trace/tracer.cc | 9 - sdk/src/trace/tracer_provider.cc | 10 +- sdk/test/trace/tracer_provider_test.cc | 27 -- sdk/test/trace/tracer_test.cc | 20 -- third_party/opentelemetry-proto/README | 2 +- .../metrics/v1/metrics_service_http.yaml | 9 - .../trace/v1/trace_service_http.yaml | 3 + .../proto/common/v1/common.proto | 66 ++--- .../proto/metrics/v1/metrics.proto | 233 ++++++------------ .../proto/resource/v1/resource.proto | 2 +- .../opentelemetry/proto/trace/v1/trace.proto | 32 +-- 18 files changed, 121 insertions(+), 450 deletions(-) delete mode 100644 exporters/otlp/recordable_test.cc delete mode 100644 third_party/opentelemetry-proto/opentelemetry/proto/collector/metrics/v1/metrics_service_http.yaml diff --git a/exporters/otlp/BUILD b/exporters/otlp/BUILD index fd1f048889..09b1516ab5 100644 --- a/exporters/otlp/BUILD +++ b/exporters/otlp/BUILD @@ -28,12 +28,3 @@ cc_library( "@com_github_opentelemetry_proto//:trace_proto_cc", ], ) - -cc_test( - name = "recordable_test", - srcs = ["recordable_test.cc"], - deps = [ - ":recordable", - "@com_google_googletest//:gtest_main", - ], -) diff --git a/exporters/otlp/CMakeLists.txt b/exporters/otlp/CMakeLists.txt index 3cfc1a5832..07311ce909 100644 --- a/exporters/otlp/CMakeLists.txt +++ b/exporters/otlp/CMakeLists.txt @@ -1,11 +1,3 @@ add_library(opentelemetry_exporter_otprotocol recordable.cc) target_link_libraries(opentelemetry_exporter_otprotocol $) - -add_executable(recordable_test recordable_test.cc) -target_link_libraries(recordable_test - ${GTEST_BOTH_LIBRARIES} - ${CMAKE_THREAD_LIBS_INIT} - opentelemetry_exporter_otprotocol - protobuf::libprotobuf) -gtest_add_tests(TARGET recordable_test TEST_PREFIX exporter. TEST_LIST recordable_test) diff --git a/exporters/otlp/recordable.cc b/exporters/otlp/recordable.cc index 1677bb6be1..d38f284577 100644 --- a/exporters/otlp/recordable.cc +++ b/exporters/otlp/recordable.cc @@ -5,23 +5,6 @@ namespace exporter { namespace otlp { -void Recordable::SetIds(trace::TraceId trace_id, - trace::SpanId span_id, - trace::SpanId parent_span_id) noexcept -{ - span_.set_trace_id(reinterpret_cast(trace_id.Id().data()), trace::TraceId::kSize); - span_.set_span_id(reinterpret_cast(span_id.Id().data()), trace::SpanId::kSize); - span_.set_parent_span_id(reinterpret_cast(parent_span_id.Id().data()), - trace::SpanId::kSize); -} - -void Recordable::SetAttribute(nostd::string_view key, - const opentelemetry::common::AttributeValue &&value) noexcept -{ - (void)key; - (void)value; -} - void Recordable::AddEvent(nostd::string_view name, core::SystemTimestamp timestamp) noexcept { (void)name; @@ -37,18 +20,6 @@ void Recordable::SetName(nostd::string_view name) noexcept { span_.set_name(name.data(), name.size()); } - -void Recordable::SetStartTime(opentelemetry::core::SystemTimestamp start_time) noexcept -{ - const uint64_t nano_unix_time = start_time.time_since_epoch().count(); - span_.set_start_time_unix_nano(nano_unix_time); -} - -void Recordable::SetDuration(std::chrono::nanoseconds duration) noexcept -{ - const uint64_t unix_end_time = span_.start_time_unix_nano() + duration.count(); - span_.set_end_time_unix_nano(unix_end_time); -} } // namespace otlp } // namespace exporter OPENTELEMETRY_END_NAMESPACE diff --git a/exporters/otlp/recordable.h b/exporters/otlp/recordable.h index 8490bc9141..5ea1edf497 100644 --- a/exporters/otlp/recordable.h +++ b/exporters/otlp/recordable.h @@ -14,23 +14,13 @@ class Recordable final : public sdk::trace::Recordable public: const proto::trace::v1::Span &span() const noexcept { return span_; } - void SetIds(trace::TraceId trace_id, - trace::SpanId span_id, - trace::SpanId parent_span_id) noexcept override; - - void SetAttribute(nostd::string_view key, - const opentelemetry::common::AttributeValue &&value) noexcept override; - + // sdk::trace::Recordable void AddEvent(nostd::string_view name, core::SystemTimestamp timestamp) noexcept override; void SetStatus(trace::CanonicalCode code, nostd::string_view description) noexcept override; void SetName(nostd::string_view name) noexcept override; - void SetStartTime(opentelemetry::core::SystemTimestamp start_time) noexcept override; - - void SetDuration(std::chrono::nanoseconds duration) noexcept override; - private: proto::trace::v1::Span span_; }; diff --git a/exporters/otlp/recordable_test.cc b/exporters/otlp/recordable_test.cc deleted file mode 100644 index b086c46b36..0000000000 --- a/exporters/otlp/recordable_test.cc +++ /dev/null @@ -1,73 +0,0 @@ -#include "exporters/otlp/recordable.h" - -#include - -OPENTELEMETRY_BEGIN_NAMESPACE -namespace exporter -{ -namespace otlp -{ -TEST(Recordable, SetIds) -{ - const trace::TraceId trace_id( - std::array( - {0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 1})); - - const trace::SpanId span_id( - std::array( - {0, 0, 0, 0, 0, 0, 0, 2})); - - const trace::SpanId parent_span_id( - std::array( - {0, 0, 0, 0, 0, 0, 0, 3})); - - Recordable rec; - - rec.SetIds(trace_id, span_id, parent_span_id); - - EXPECT_EQ(rec.span().trace_id(), - std::string(reinterpret_cast(trace_id.Id().data()), trace::TraceId::kSize)); - EXPECT_EQ(rec.span().span_id(), - std::string(reinterpret_cast(span_id.Id().data()), trace::SpanId::kSize)); - EXPECT_EQ(rec.span().parent_span_id(), - std::string(reinterpret_cast(parent_span_id.Id().data()), trace::SpanId::kSize)); -} - -TEST(Recordable, SetName) -{ - Recordable rec; - nostd::string_view name = "TestSpan"; - rec.SetName(name); - EXPECT_EQ(rec.span().name(), name); -} - -TEST(Recordable, SetStartTime) -{ - Recordable rec; - std::chrono::system_clock::time_point start_time = std::chrono::system_clock::now(); - core::SystemTimestamp start_timestamp(start_time); - - uint64_t unix_start = - std::chrono::duration_cast(start_time.time_since_epoch()).count(); - - rec.SetStartTime(start_timestamp); - EXPECT_EQ(rec.span().start_time_unix_nano(), unix_start); -} - -TEST(Recordable, SetDuration) -{ - Recordable rec; - // Start time is 0 - core::SystemTimestamp start_timestamp; - - std::chrono::nanoseconds duration(10); - uint64_t unix_end = duration.count(); - - rec.SetStartTime(start_timestamp); - rec.SetDuration(duration); - - EXPECT_EQ(rec.span().end_time_unix_nano(), unix_end); -} -} // namespace otlp -} // namespace exporter -OPENTELEMETRY_END_NAMESPACE diff --git a/sdk/include/opentelemetry/sdk/trace/tracer.h b/sdk/include/opentelemetry/sdk/trace/tracer.h index 2d0afde498..577785878a 100644 --- a/sdk/include/opentelemetry/sdk/trace/tracer.h +++ b/sdk/include/opentelemetry/sdk/trace/tracer.h @@ -2,7 +2,6 @@ #include "opentelemetry/sdk/common/atomic_shared_ptr.h" #include "opentelemetry/sdk/trace/processor.h" -#include "opentelemetry/sdk/trace/samplers/always_on.h" #include "opentelemetry/trace/tracer.h" #include "opentelemetry/version.h" @@ -21,8 +20,7 @@ class Tracer final : public trace_api::Tracer, public std::enable_shared_from_th * @param processor The span processor for this tracer. This must not be a * nullptr. */ - explicit Tracer(std::shared_ptr processor, - std::shared_ptr sampler = std::make_shared()) noexcept; + explicit Tracer(std::shared_ptr processor) noexcept : processor_{processor} {} /** * Set the span processor associated with this tracer. @@ -37,12 +35,6 @@ class Tracer final : public trace_api::Tracer, public std::enable_shared_from_th */ std::shared_ptr GetProcessor() const noexcept; - /** - * Obtain the sampler associated with this tracer. - * @return The sampler for this tracer. - */ - std::shared_ptr GetSampler() const noexcept; - nostd::unique_ptr StartSpan( nostd::string_view name, const trace_api::KeyValueIterable &attributes, @@ -54,7 +46,6 @@ class Tracer final : public trace_api::Tracer, public std::enable_shared_from_th private: opentelemetry::sdk::AtomicSharedPtr processor_; - const std::shared_ptr sampler_; }; } // namespace trace } // namespace sdk diff --git a/sdk/include/opentelemetry/sdk/trace/tracer_provider.h b/sdk/include/opentelemetry/sdk/trace/tracer_provider.h index 6581ad46be..d78fed02fc 100644 --- a/sdk/include/opentelemetry/sdk/trace/tracer_provider.h +++ b/sdk/include/opentelemetry/sdk/trace/tracer_provider.h @@ -6,7 +6,6 @@ #include "opentelemetry/nostd/shared_ptr.h" #include "opentelemetry/sdk/trace/processor.h" -#include "opentelemetry/sdk/trace/samplers/always_on.h" #include "opentelemetry/sdk/trace/tracer.h" #include "opentelemetry/trace/tracer_provider.h" @@ -19,15 +18,11 @@ class TracerProvider final : public opentelemetry::trace::TracerProvider { public: /** - * Initialize a new tracer provider with a specified sampler + * Initialize a new tracer provider. * @param processor The span processor for this tracer provider. This must * not be a nullptr. - * @param sampler The sampler for this tracer provider. This must - * not be a nullptr. */ - explicit TracerProvider( - std::shared_ptr processor, - std::shared_ptr sampler = std::make_shared()) noexcept; + explicit TracerProvider(std::shared_ptr processor) noexcept; opentelemetry::nostd::shared_ptr GetTracer( nostd::string_view library_name, @@ -46,16 +41,9 @@ class TracerProvider final : public opentelemetry::trace::TracerProvider */ std::shared_ptr GetProcessor() const noexcept; - /** - * Obtain the sampler associated with this tracer provider. - * @return The span processor for this tracer provider. - */ - std::shared_ptr GetSampler() const noexcept; - private: opentelemetry::sdk::AtomicSharedPtr processor_; std::shared_ptr tracer_; - const std::shared_ptr sampler_; }; } // namespace trace } // namespace sdk diff --git a/sdk/src/trace/tracer.cc b/sdk/src/trace/tracer.cc index 959fa35259..311459060b 100644 --- a/sdk/src/trace/tracer.cc +++ b/sdk/src/trace/tracer.cc @@ -9,10 +9,6 @@ namespace sdk { namespace trace { -Tracer::Tracer(std::shared_ptr processor, std::shared_ptr sampler) noexcept - : processor_{processor}, sampler_{sampler} -{} - void Tracer::SetProcessor(std::shared_ptr processor) noexcept { processor_.store(processor); @@ -23,11 +19,6 @@ std::shared_ptr Tracer::GetProcessor() const noexcept return processor_.load(); } -std::shared_ptr Tracer::GetSampler() const noexcept -{ - return sampler_; -} - nostd::unique_ptr Tracer::StartSpan( nostd::string_view name, const trace_api::KeyValueIterable &attributes, diff --git a/sdk/src/trace/tracer_provider.cc b/sdk/src/trace/tracer_provider.cc index 32144c4c6b..3268207510 100644 --- a/sdk/src/trace/tracer_provider.cc +++ b/sdk/src/trace/tracer_provider.cc @@ -5,9 +5,8 @@ namespace sdk { namespace trace { -TracerProvider::TracerProvider(std::shared_ptr processor, - std::shared_ptr sampler) noexcept - : processor_{processor}, tracer_(new Tracer(std::move(processor))), sampler_(sampler) +TracerProvider::TracerProvider(std::shared_ptr processor) noexcept + : processor_{processor}, tracer_(new Tracer(std::move(processor))) {} opentelemetry::nostd::shared_ptr TracerProvider::GetTracer( @@ -29,11 +28,6 @@ std::shared_ptr TracerProvider::GetProcessor() const noexcept { return processor_.load(); } - -std::shared_ptr TracerProvider::GetSampler() const noexcept -{ - return sampler_; -} } // namespace trace } // namespace sdk OPENTELEMETRY_END_NAMESPACE diff --git a/sdk/test/trace/tracer_provider_test.cc b/sdk/test/trace/tracer_provider_test.cc index 399ea6d453..90dbe7c747 100644 --- a/sdk/test/trace/tracer_provider_test.cc +++ b/sdk/test/trace/tracer_provider_test.cc @@ -1,6 +1,4 @@ #include "opentelemetry/sdk/trace/tracer_provider.h" -#include "opentelemetry/sdk/trace/samplers/always_off.h" -#include "opentelemetry/sdk/trace/samplers/always_on.h" #include "opentelemetry/sdk/trace/simple_processor.h" #include "opentelemetry/sdk/trace/tracer.h" @@ -29,28 +27,3 @@ TEST(TracerProvider, GetTracer) ASSERT_NE(nullptr, sdkTracer); ASSERT_EQ(processor, sdkTracer->GetProcessor()); } - -TEST(TracerProvider, GetSampler) -{ - std::shared_ptr processor1(new SimpleSpanProcessor(nullptr)); - - // Create a TracerProvicer with a default AlwaysOnSampler. - TracerProvider tf1(processor1); - auto t1 = tf1.GetSampler(); - auto t2 = tf1.GetSampler(); - ASSERT_NE(nullptr, t1); - ASSERT_NE(nullptr, t2); - - // Should return the same sampler each time. - ASSERT_EQ(t1, t2); - - // Should be AlwaysOnSampler - ASSERT_EQ("AlwaysOnSampler", t2->GetDescription()); - - // Create a TracerProvicer with a custom AlwaysOffSampler. - std::shared_ptr processor2(new SimpleSpanProcessor(nullptr)); - TracerProvider tf2(processor2, std::make_shared()); - auto t3 = tf2.GetSampler(); - - ASSERT_EQ("AlwaysOffSampler", t3->GetDescription()); -} diff --git a/sdk/test/trace/tracer_test.cc b/sdk/test/trace/tracer_test.cc index 2c6b891818..056227aa8d 100644 --- a/sdk/test/trace/tracer_test.cc +++ b/sdk/test/trace/tracer_test.cc @@ -1,7 +1,5 @@ #include "opentelemetry/sdk/trace/tracer.h" #include "opentelemetry/sdk/trace/simple_processor.h" -#include "opentelemetry/sdk/trace/samplers/always_on.h" -#include "opentelemetry/sdk/trace/samplers/always_off.h" #include "opentelemetry/sdk/trace/span_data.h" #include @@ -141,21 +139,3 @@ TEST(Tracer, StartSpanWithAttributes) ASSERT_EQ(1, span_data2->GetAttributes().size()); ASSERT_EQ(3.0, nostd::get(span_data2->GetAttributes().at("attr3"))); } - - -TEST(Tracer, GetSampler) -{ - // Create a Tracer with a default AlwaysOnSampler - std::shared_ptr processor_1(new SimpleSpanProcessor(nullptr)); - std::shared_ptr tracer_on(new Tracer(std::move(processor_1))); - - auto t1 = tracer_on->GetSampler(); - ASSERT_EQ("AlwaysOnSampler", t1->GetDescription()); - - // Create a Tracer with a AlwaysOffSampler - std::shared_ptr processor_2(new SimpleSpanProcessor(nullptr)); - std::shared_ptr tracer_off(new Tracer(std::move(processor_2), std::make_shared())); - - auto t2 = tracer_off->GetSampler(); - ASSERT_EQ("AlwaysOffSampler", t2->GetDescription()); -} diff --git a/third_party/opentelemetry-proto/README b/third_party/opentelemetry-proto/README index e32d7de183..dfd3a68e21 100644 --- a/third_party/opentelemetry-proto/README +++ b/third_party/opentelemetry-proto/README @@ -1,2 +1,2 @@ From: https://github.com/open-telemetry/opentelemetry-proto -Commit: e43e1abc40428a6ee98e3bfd79bec1dfa2ed18cd +Commit: d496c80b353bc4a4f754ae686b59ca3c41de0946 diff --git a/third_party/opentelemetry-proto/opentelemetry/proto/collector/metrics/v1/metrics_service_http.yaml b/third_party/opentelemetry-proto/opentelemetry/proto/collector/metrics/v1/metrics_service_http.yaml deleted file mode 100644 index bc5f9ff241..0000000000 --- a/third_party/opentelemetry-proto/opentelemetry/proto/collector/metrics/v1/metrics_service_http.yaml +++ /dev/null @@ -1,9 +0,0 @@ -# This is an API configuration to generate an HTTP/JSON -> gRPC gateway for the -# OpenTelemetry service using github.com/grpc-ecosystem/grpc-gateway. -type: google.api.Service -config_version: 3 -http: - rules: - - selector: opentelemetry.proto.collector.metrics.v1.MetricsService.Export - post: /v1/metrics - body: "*" diff --git a/third_party/opentelemetry-proto/opentelemetry/proto/collector/trace/v1/trace_service_http.yaml b/third_party/opentelemetry-proto/opentelemetry/proto/collector/trace/v1/trace_service_http.yaml index 10eae48d51..7754e5ff12 100644 --- a/third_party/opentelemetry-proto/opentelemetry/proto/collector/trace/v1/trace_service_http.yaml +++ b/third_party/opentelemetry-proto/opentelemetry/proto/collector/trace/v1/trace_service_http.yaml @@ -7,3 +7,6 @@ http: - selector: opentelemetry.proto.collector.trace.v1.TraceService.Export post: /v1/trace body: "*" + - selector: opentelemetry.proto.collector.metrics.v1.MetricsService.Export + post: /v1/trace + body: "*" \ No newline at end of file diff --git a/third_party/opentelemetry-proto/opentelemetry/proto/common/v1/common.proto b/third_party/opentelemetry-proto/opentelemetry/proto/common/v1/common.proto index dc67e43fb6..b3b1852459 100644 --- a/third_party/opentelemetry-proto/opentelemetry/proto/common/v1/common.proto +++ b/third_party/opentelemetry-proto/opentelemetry/proto/common/v1/common.proto @@ -21,57 +21,35 @@ option java_package = "io.opentelemetry.proto.common.v1"; option java_outer_classname = "CommonProto"; option go_package = "github.com/open-telemetry/opentelemetry-proto/gen/go/common/v1"; -// AnyValue is used to represent any type of attribute value. AnyValue may contain a -// primitive value such as a string or integer or it may contain an arbitrary nested -// object containing arrays, key-value lists and primitives. -message AnyValue { - // The value is one of the listed fields. It is valid for all values to be unspecified - // in which case this AnyValue is considered to be "null". - oneof value { - string string_value = 1; - bool bool_value = 2; - int64 int_value = 3; - double double_value = 4; - ArrayValue array_value = 5; - KeyValueList kvlist_value = 6; - } -} +// AttributeKeyValue is a key-value pair that is used to store Span attributes, Link +// attributes, etc. +message AttributeKeyValue { + // ValueType is the enumeration of possible types that value can have. + enum ValueType { + STRING = 0; + INT = 1; + DOUBLE = 2; + BOOL = 3; + }; + + // key part of the key-value pair. + string key = 1; -// ArrayValue is a list of AnyValue messages. We need ArrayValue as a message -// since oneof in AnyValue does not allow repeated fields. -message ArrayValue { - // Array of values. The array may be empty (contain 0 elements). - repeated AnyValue values = 1; -} + // type of the value. + ValueType type = 2; -// KeyValueList is a list of KeyValue messages. We need KeyValueList as a message -// since `oneof` in AnyValue does not allow repeated fields. Everywhere else where we need -// a list of KeyValue messages (e.g. in Span) we use `repeated KeyValue` directly to -// avoid unnecessary extra wrapping (which slows down the protocol). The 2 approaches -// are semantically equivalent. -message KeyValueList { - // A collection of key/value pairs of key-value pairs. The list may be empty (may - // contain 0 elements). - repeated KeyValue values = 1; -} + // Only one of the following fields is supposed to contain data (determined by `type` field). + // This is deliberately not using Protobuf `oneof` for performance reasons (verified by benchmarks). -// KeyValue is a key-value pair that is used to store Span attributes, Link -// attributes, etc. -message KeyValue { - string key = 1; - AnyValue value = 2; + string string_value = 3; + int64 int_value = 4; + double double_value = 5; + bool bool_value = 6; } // StringKeyValue is a pair of key/value strings. This is the simpler (and faster) version -// of KeyValue that only supports string values. +// of AttributeKeyValue that only supports string values. message StringKeyValue { string key = 1; string value = 2; } - -// InstrumentationLibrary is a message representing the instrumentation library information -// such as the fully qualified name and version. -message InstrumentationLibrary { - string name = 1; - string version = 2; -} diff --git a/third_party/opentelemetry-proto/opentelemetry/proto/metrics/v1/metrics.proto b/third_party/opentelemetry-proto/opentelemetry/proto/metrics/v1/metrics.proto index 1d458824fe..a15a7b6c2b 100644 --- a/third_party/opentelemetry-proto/opentelemetry/proto/metrics/v1/metrics.proto +++ b/third_party/opentelemetry-proto/opentelemetry/proto/metrics/v1/metrics.proto @@ -24,24 +24,14 @@ option java_package = "io.opentelemetry.proto.metrics.v1"; option java_outer_classname = "MetricsProto"; option go_package = "github.com/open-telemetry/opentelemetry-proto/gen/go/metrics/v1"; -// A collection of InstrumentationLibraryMetrics from a Resource. +// A collection of metrics from a Resource. message ResourceMetrics { - // The resource for the metrics in this message. - // If this field is not set then no resource info is known. - opentelemetry.proto.resource.v1.Resource resource = 1; - // A list of metrics that originate from a resource. - repeated InstrumentationLibraryMetrics instrumentation_library_metrics = 2; -} + repeated Metric metrics = 1; -// A collection of Metrics produced by an InstrumentationLibrary. -message InstrumentationLibraryMetrics { - // The instrumentation library information for the metrics in this message. - // If this field is not set then no library info is known. - opentelemetry.proto.common.v1.InstrumentationLibrary instrumentation_library = 1; - - // A list of metrics that originate from an instrumentation library. - repeated Metric metrics = 2; + // The resource for the metrics in this message. + // If this field is not set then no resource info is known. + opentelemetry.proto.resource.v1.Resource resource = 2; } // Defines a Metric which has one or more timeseries. @@ -100,10 +90,10 @@ message Metric { // Data is a list of one or more DataPoints for a single metric. Only one of the // following fields is used for the data, depending on the type of the metric defined // by MetricDescriptor.type field. - repeated Int64DataPoint int64_data_points = 2; - repeated DoubleDataPoint double_data_points = 3; - repeated HistogramDataPoint histogram_data_points = 4; - repeated SummaryDataPoint summary_data_points = 5; + repeated Int64DataPoint int64_datapoints = 2; + repeated DoubleDataPoint double_datapoints = 3; + repeated HistogramDataPoint histogram_datapoints = 4; + repeated SummaryDataPoint summary_datapoints = 5; } // Defines a metric type and its schema. @@ -118,36 +108,49 @@ message MetricDescriptor { // described by http://unitsofmeasure.org/ucum.html. string unit = 3; - // Type is the type of values a metric has. + // Type of the metric. It describes how the data is reported. + // + // A gauge is an instantaneous measurement of a value. + // + // A counter/cumulative measurement is a value accumulated over a time + // interval. In a time series, cumulative measurements should have the same + // start time, increasing values, until an event resets the cumulative value + // to zero and sets a new start time for the subsequent points. enum Type { - // INVALID_TYPE is the default Type, it MUST not be used. - INVALID_TYPE = 0; - - // INT64 values are signed 64-bit integers. - // - // A Metric of this Type MUST store its values as Int64DataPoint. - INT64 = 1; - - // MONOTONIC_INT64 values are monotonically increasing signed 64-bit - // integers. - // - // A Metric of this Type MUST store its values as Int64DataPoint. - MONOTONIC_INT64 = 2; - - // DOUBLE values are double-precision floating-point numbers. - // - // A Metric of this Type MUST store its values as DoubleDataPoint. - DOUBLE = 3; - - // MONOTONIC_DOUBLE values are monotonically increasing double-precision - // floating-point numbers. - // - // A Metric of this Type MUST store its values as DoubleDataPoint. - MONOTONIC_DOUBLE = 4; - - // Histogram measurement. - // Corresponding values are stored in HistogramDataPoint. - HISTOGRAM = 5; + // Do not use this default value. + UNSPECIFIED = 0; + + // Integer gauge. The value can go both up and down over time. + // Corresponding values are stored in Int64DataPoint. + GAUGE_INT64 = 1; + + // Floating point gauge. The value can go both up and down over time. + // Corresponding values are stored in DoubleDataPoint. + GAUGE_DOUBLE = 2; + + // Histogram gauge measurement. + // Used in scenarios like a snapshot of time that current items in a queue + // have spent there. + // Corresponding values are stored in HistogramDataPoint. The count and sum of the + // histogram can go both up and down over time. Recorded values are always >= 0. + GAUGE_HISTOGRAM = 3; + + // Integer counter measurement. The value cannot decrease; if value is reset then + // start_time_unixnano should also be reset. + // Corresponding values are stored in Int64DataPoint. + COUNTER_INT64 = 4; + + // Floating point counter measurement. The value cannot decrease, if + // resets then the start_time_unixnano should also be reset. + // Recorded values are always >= 0. + // Corresponding values are stored in DoubleDataPoint. + COUNTER_DOUBLE = 5; + + // Histogram cumulative measurement. + // Corresponding values are stored in HistogramDataPoint. The count and sum of the + // histogram cannot decrease; if values are reset then start_time_unixnano + // should also be reset to the new start timestamp. + CUMULATIVE_HISTOGRAM = 6; // Summary value. Some frameworks implemented Histograms as a summary of observations // (usually things like request durations and response sizes). While it @@ -155,88 +158,13 @@ message MetricDescriptor { // values, it calculates configurable percentiles over a sliding time // window. // Corresponding values are stored in SummaryDataPoint. - SUMMARY = 6; + SUMMARY = 7; } - - // type is the type of values this metric has. Type type = 4; - // Temporality is the temporal quality values of a metric have. It - // describes how those values relate to the time interval over which they - // are reported. - enum Temporality { - // INVALID_TEMPORALITY is the default Temporality, it MUST not be - // used. - INVALID_TEMPORALITY = 0; - - // INSTANTANEOUS is a metric whose values are measured at a particular - // instant. The values are not aggregated over any time interval and are - // unique per timestamp. As such, these metrics are not expected to have - // an associated start time. - INSTANTANEOUS = 1; - - // DELTA is a metric whose values are the aggregation of measurements - // made over a time interval. Successive metrics contain aggregation of - // values from continuous and non-overlapping intervals. - // - // The values for a DELTA metric are based only on the time interval - // associated with one measurement cycle. There is no dependency on - // previous measurements like is the case for CUMULATIVE metrics. - // - // For example, consider a system measuring the number of requests that - // it receives and reports the sum of these requests every second as a - // DELTA metric: - // - // 1. The system starts receiving at time=t_0. - // 2. A request is received, the system measures 1 request. - // 3. A request is received, the system measures 1 request. - // 4. A request is received, the system measures 1 request. - // 5. The 1 second collection cycle ends. A metric is exported for the - // number of requests received over the interval of time t_0 to - // t_0+1 with a value of 3. - // 6. A request is received, the system measures 1 request. - // 7. A request is received, the system measures 1 request. - // 8. The 1 second collection cycle ends. A metric is exported for the - // number of requests received over the interval of time t_0+1 to - // t_0+2 with a value of 2. - DELTA = 2; - - // CUMULATIVE is a metric whose values are the aggregation of - // successively made measurements from a fixed start time until the last - // reported measurement. This means that current values of a CUMULATIVE - // metric depend on all previous measurements since the start time. - // Because of this, the sender is required to retain this state in some - // form. If this state is lost or invalidated, the CUMULATIVE metric - // values MUST be reset and a new fixed start time following the last - // reported measurement time sent MUST be used. - // - // For example, consider a system measuring the number of requests that - // it receives and reports the sum of these requests every second as a - // CUMULATIVE metric: - // - // 1. The system starts receiving at time=t_0. - // 2. A request is received, the system measures 1 request. - // 3. A request is received, the system measures 1 request. - // 4. A request is received, the system measures 1 request. - // 5. The 1 second collection cycle ends. A metric is exported for the - // number of requests received over the interval of time t_0 to - // t_0+1 with a value of 3. - // 6. A request is received, the system measures 1 request. - // 7. A request is received, the system measures 1 request. - // 8. The 1 second collection cycle ends. A metric is exported for the - // number of requests received over the interval of time t_0 to - // t_0+2 with a value of 5. - // 9. The system experiences a fault and loses state. - // 10. The system recovers and resumes receiving at time=t_1. - // 11. A request is received, the system measures 1 request. - // 12. The 1 second collection cycle ends. A metric is exported for the - // number of requests received over the interval of time t_1 to - // t_0+1 with a value of 1. - CUMULATIVE = 3; - } - - // temporality is the Temporality of values this metric has. - Temporality temporality = 5; + // The set of labels associated with the metric descriptor. Labels in this list apply to + // all data points. + repeated opentelemetry.proto.common.v1.StringKeyValue labels = 5; } // Int64DataPoint is a single data point in a timeseries that describes the time-varying @@ -245,20 +173,20 @@ message Int64DataPoint { // The set of labels that uniquely identify this timeseries. repeated opentelemetry.proto.common.v1.StringKeyValue labels = 1; - // start_time_unix_nano is the time when the cumulative value was reset to zero. + // start_time_unixnano is the time when the cumulative value was reset to zero. // This is used for Counter type only. For Gauge the value is not specified and // defaults to 0. // - // The cumulative value is over the time interval (start_time_unix_nano, time_unix_nano]. + // The cumulative value is over the time interval (start_time_unixnano, timestamp_unixnano]. // Value is UNIX Epoch time in nanoseconds since 00:00:00 UTC on 1 January 1970. // // Value of 0 indicates that the timestamp is unspecified. In that case the timestamp // may be decided by the backend. - fixed64 start_time_unix_nano = 2; + fixed64 start_time_unixnano = 2; - // time_unix_nano is the moment when this value was recorded. + // timestamp_unixnano is the moment when this value was recorded. // Value is UNIX Epoch time in nanoseconds since 00:00:00 UTC on 1 January 1970. - fixed64 time_unix_nano = 3; + fixed64 timestamp_unixnano = 3; // value itself. int64 value = 4; @@ -270,20 +198,20 @@ message DoubleDataPoint { // The set of labels that uniquely identify this timeseries. repeated opentelemetry.proto.common.v1.StringKeyValue labels = 1; - // start_time_unix_nano is the time when the cumulative value was reset to zero. + // start_time_unixnano is the time when the cumulative value was reset to zero. // This is used for Counter type only. For Gauge the value is not specified and // defaults to 0. // - // The cumulative value is over the time interval (start_time_unix_nano, time_unix_nano]. + // The cumulative value is over the time interval (start_time_unixnano, timestamp_unixnano]. // Value is UNIX Epoch time in nanoseconds since 00:00:00 UTC on 1 January 1970. // // Value of 0 indicates that the timestamp is unspecified. In that case the timestamp // may be decided by the backend. - fixed64 start_time_unix_nano = 2; + fixed64 start_time_unixnano = 2; - // time_unix_nano is the moment when this value was recorded. + // timestamp_unixnano is the moment when this value was recorded. // Value is UNIX Epoch time in nanoseconds since 00:00:00 UTC on 1 January 1970. - fixed64 time_unix_nano = 3; + fixed64 timestamp_unixnano = 3; // value itself. double value = 4; @@ -296,19 +224,19 @@ message HistogramDataPoint { // The set of labels that uniquely identify this timeseries. repeated opentelemetry.proto.common.v1.StringKeyValue labels = 1; - // start_time_unix_nano is the time when the cumulative value was reset to zero. + // start_time_unixnano is the time when the cumulative value was reset to zero. // - // The cumulative value is over the time interval (start_time_unix_nano, time_unix_nano]. + // The cumulative value is over the time interval (start_time_unixnano, timestamp_unixnano]. // Value is UNIX Epoch time in nanoseconds since 00:00:00 UTC on 1 January 1970. // // Value of 0 indicates that the timestamp is unspecified. In that case the timestamp // may be decided by the backend. // Note: this field is always unspecified and ignored if MetricDescriptor.type==GAUGE_HISTOGRAM. - fixed64 start_time_unix_nano = 2; + fixed64 start_time_unixnano = 2; - // time_unix_nano is the moment when this value was recorded. + // timestamp_unixnano is the moment when this value was recorded. // Value is UNIX Epoch time in nanoseconds since 00:00:00 UTC on 1 January 1970. - fixed64 time_unix_nano = 3; + fixed64 timestamp_unixnano = 3; // count is the number of values in the population. Must be non-negative. This value // must be equal to the sum of the "count" fields in buckets if a histogram is provided. @@ -334,9 +262,9 @@ message HistogramDataPoint { // the defined bounds. double value = 1; - // time_unix_nano is the moment when this exemplar was recorded. + // timestamp_unixnano is the moment when this exemplar was recorded. // Value is UNIX Epoch time in nanoseconds since 00:00:00 UTC on 1 January 1970. - fixed64 time_unix_nano = 2; + fixed64 timestamp_unixnano = 2; // exemplar_attachments are contextual information about the example value. // Keys in this list must be unique. @@ -390,18 +318,18 @@ message SummaryDataPoint { // The set of labels that uniquely identify this timeseries. repeated opentelemetry.proto.common.v1.StringKeyValue labels = 1; - // start_time_unix_nano is the time when the cumulative value was reset to zero. + // start_time_unixnano is the time when the cumulative value was reset to zero. // - // The cumulative value is over the time interval (start_time_unix_nano, time_unix_nano]. + // The cumulative value is over the time interval (start_time_unixnano, timestamp_unixnano]. // Value is UNIX Epoch time in nanoseconds since 00:00:00 UTC on 1 January 1970. // // Value of 0 indicates that the timestamp is unspecified. In that case the timestamp // may be decided by the backend. - fixed64 start_time_unix_nano = 2; + fixed64 start_time_unixnano = 2; - // time_unix_nano is the moment when this value was recorded. + // timestamp_unixnano is the moment when this value was recorded. // Value is UNIX Epoch time in nanoseconds since 00:00:00 UTC on 1 January 1970. - fixed64 time_unix_nano = 3; + fixed64 timestamp_unixnano = 3; // The total number of recorded values since start_time. Optional since // some systems don't expose this. @@ -412,13 +340,6 @@ message SummaryDataPoint { double sum = 5; // Represents the value at a given percentile of a distribution. - // - // To record Min and Max values following conventions are used: - // - The 100th percentile is equivalent to the maximum value observed. - // - The 0th percentile is equivalent to the minimum value observed. - // - // See the following issue for more context: - // https://github.com/open-telemetry/opentelemetry-proto/issues/125 message ValueAtPercentile { // The percentile of a distribution. Must be in the interval // [0.0, 100.0]. diff --git a/third_party/opentelemetry-proto/opentelemetry/proto/resource/v1/resource.proto b/third_party/opentelemetry-proto/opentelemetry/proto/resource/v1/resource.proto index fa5d97c6f8..a9e1711af4 100644 --- a/third_party/opentelemetry-proto/opentelemetry/proto/resource/v1/resource.proto +++ b/third_party/opentelemetry-proto/opentelemetry/proto/resource/v1/resource.proto @@ -26,7 +26,7 @@ option go_package = "github.com/open-telemetry/opentelemetry-proto/gen/go/resour // Resource information. message Resource { // Set of labels that describe the resource. - repeated opentelemetry.proto.common.v1.KeyValue attributes = 1; + repeated opentelemetry.proto.common.v1.AttributeKeyValue attributes = 1; // dropped_attributes_count is the number of dropped attributes. If the value is 0, then // no attributes were dropped. diff --git a/third_party/opentelemetry-proto/opentelemetry/proto/trace/v1/trace.proto b/third_party/opentelemetry-proto/opentelemetry/proto/trace/v1/trace.proto index 5a2350fdee..7f0e4a75c0 100644 --- a/third_party/opentelemetry-proto/opentelemetry/proto/trace/v1/trace.proto +++ b/third_party/opentelemetry-proto/opentelemetry/proto/trace/v1/trace.proto @@ -24,23 +24,13 @@ option java_package = "io.opentelemetry.proto.trace.v1"; option java_outer_classname = "TraceProto"; option go_package = "github.com/open-telemetry/opentelemetry-proto/gen/go/trace/v1"; -// A collection of InstrumentationLibrarySpans from a Resource. +// A collection of spans from a Resource. message ResourceSpans { // The resource for the spans in this message. // If this field is not set then no resource info is known. opentelemetry.proto.resource.v1.Resource resource = 1; - // A list of InstrumentationLibrarySpans that originate from a resource. - repeated InstrumentationLibrarySpans instrumentation_library_spans = 2; -} - -// A collection of Spans produced by an InstrumentationLibrary. -message InstrumentationLibrarySpans { - // The instrumentation library information for the spans in this message. - // If this field is not set then no library info is known. - opentelemetry.proto.common.v1.InstrumentationLibrary instrumentation_library = 1; - - // A list of Spans that originate from an instrumentation library. + // A list of Spans that originate from a resource. repeated Span spans = 2; } @@ -133,21 +123,21 @@ message Span { // and `SERVER` (callee) to identify queueing latency associated with the span. SpanKind kind = 6; - // start_time_unix_nano is the start time of the span. On the client side, this is the time + // start_time_unixnano is the start time of the span. On the client side, this is the time // kept by the local machine where the span execution starts. On the server side, this // is the time when the server's application handler starts running. // Value is UNIX Epoch time in nanoseconds since 00:00:00 UTC on 1 January 1970. // // This field is semantically required and it is expected that end_time >= start_time. - fixed64 start_time_unix_nano = 7; + fixed64 start_time_unixnano = 7; - // end_time_unix_nano is the end time of the span. On the client side, this is the time + // end_time_unixnano is the end time of the span. On the client side, this is the time // kept by the local machine where the span execution ends. On the server side, this // is the time when the server application handler stops running. // Value is UNIX Epoch time in nanoseconds since 00:00:00 UTC on 1 January 1970. // // This field is semantically required and it is expected that end_time >= start_time. - fixed64 end_time_unix_nano = 8; + fixed64 end_time_unixnano = 8; // attributes is a collection of key/value pairs. The value can be a string, // an integer, a double or the Boolean values `true` or `false`. Note, global attributes @@ -157,7 +147,7 @@ message Span { // "/http/server_latency": 300 // "abc.com/myattribute": true // "abc.com/score": 10.239 - repeated opentelemetry.proto.common.v1.KeyValue attributes = 9; + repeated opentelemetry.proto.common.v1.AttributeKeyValue attributes = 9; // dropped_attributes_count is the number of attributes that were discarded. Attributes // can be discarded because their keys are too long or because there are too many @@ -167,15 +157,15 @@ message Span { // Event is a time-stamped annotation of the span, consisting of user-supplied // text description and key-value pairs. message Event { - // time_unix_nano is the time the event occurred. - fixed64 time_unix_nano = 1; + // time_unixnano is the time the event occurred. + fixed64 time_unixnano = 1; // name of the event. // This field is semantically required to be set to non-empty string. string name = 2; // attributes is a collection of attribute key/value pairs on the event. - repeated opentelemetry.proto.common.v1.KeyValue attributes = 3; + repeated opentelemetry.proto.common.v1.AttributeKeyValue attributes = 3; // dropped_attributes_count is the number of dropped attributes. If the value is 0, // then no attributes were dropped. @@ -205,7 +195,7 @@ message Span { string trace_state = 3; // attributes is a collection of attribute key/value pairs on the link. - repeated opentelemetry.proto.common.v1.KeyValue attributes = 4; + repeated opentelemetry.proto.common.v1.AttributeKeyValue attributes = 4; // dropped_attributes_count is the number of dropped attributes. If the value is 0, // then no attributes were dropped.