From 7480c46cc9e5d25fab952d981c1417bf58f2a166 Mon Sep 17 00:00:00 2001 From: Nadia Ciobanu Date: Wed, 19 Aug 2020 12:52:57 -0700 Subject: [PATCH 1/5] Simplify TraceState tests (#283) --- api/test/trace/trace_state_test.cc | 30 +++++++++++++++--------------- 1 file changed, 15 insertions(+), 15 deletions(-) diff --git a/api/test/trace/trace_state_test.cc b/api/test/trace/trace_state_test.cc index 6f50142b46..afe4493993 100644 --- a/api/test/trace/trace_state_test.cc +++ b/api/test/trace/trace_state_test.cc @@ -23,10 +23,10 @@ TEST(EntryTest, KeyValueConstruction) TraceState::Entry e(key, val); EXPECT_EQ(key.size(), e.GetKey().size()); - EXPECT_EQ(strcmp(key.data(), e.GetKey().data()), 0); + EXPECT_EQ(key, e.GetKey()); EXPECT_EQ(val.size(), e.GetValue().size()); - EXPECT_EQ(strcmp(val.data(), e.GetValue().data()), 0); + EXPECT_EQ(val, e.GetValue()); } // Test copy constructor @@ -34,8 +34,8 @@ TEST(EntryTest, Copy) { TraceState::Entry e("test_key", "test_value"); TraceState::Entry copy(e); - EXPECT_EQ(strcmp(copy.GetKey().data(), e.GetKey().data()), 0); - EXPECT_EQ(strcmp(copy.GetValue().data(), e.GetValue().data()), 0); + EXPECT_EQ(copy.GetKey(), e.GetKey()); + EXPECT_EQ(copy.GetValue(), e.GetValue()); } // Test assignment operator @@ -44,8 +44,8 @@ TEST(EntryTest, Assignment) TraceState::Entry e("test_key", "test_value"); TraceState::Entry empty; empty = e; - EXPECT_EQ(strcmp(empty.GetKey().data(), e.GetKey().data()), 0); - EXPECT_EQ(strcmp(empty.GetValue().data(), e.GetValue().data()), 0); + EXPECT_EQ(empty.GetKey(), e.GetKey()); + EXPECT_EQ(empty.GetValue(), e.GetValue()); } TEST(EntryTest, SetValue) @@ -55,7 +55,7 @@ TEST(EntryTest, SetValue) e.SetValue(new_val); EXPECT_EQ(new_val.size(), e.GetValue().size()); - EXPECT_EQ(strcmp(new_val.data(), e.GetValue().data()), 0); + EXPECT_EQ(new_val, e.GetValue()); } // -------------------------- TraceState class tests --------------------------- @@ -65,9 +65,9 @@ TEST(TraceStateTest, DefaultConstruction) TraceState s; opentelemetry::nostd::string_view return_val = ""; EXPECT_FALSE(s.Get("missing_key", return_val)); - EXPECT_EQ(return_val.data(), ""); + EXPECT_EQ(return_val, ""); EXPECT_TRUE(s.Empty()); - EXPECT_EQ(0, s.Entries().size()); + EXPECT_EQ(s.Entries().size(), 0); } TEST(TraceStateTest, Set) @@ -84,8 +84,8 @@ TEST(TraceStateTest, Set) opentelemetry::nostd::span entries = s.Entries(); EXPECT_EQ(entries.size(), 1); - EXPECT_EQ(entries[0].GetKey().data(), key); - EXPECT_EQ(entries[0].GetValue().data(), val); + EXPECT_EQ(entries[0].GetKey(), key); + EXPECT_EQ(entries[0].GetValue(), val); } TEST(TraceStateTest, Get) @@ -105,12 +105,12 @@ TEST(TraceStateTest, Get) for (int i = 0; i < kNumPairs; i++) { EXPECT_TRUE(s.Get(keys[i], return_val)); - EXPECT_EQ(return_val.data(), values[i]); + EXPECT_EQ(return_val, values[i]); return_val = ""; } EXPECT_FALSE(s.Get("fake_key", return_val)); - EXPECT_EQ(return_val.data(), ""); + EXPECT_EQ(return_val, ""); } TEST(TraceStateTest, Empty) @@ -137,8 +137,8 @@ TEST(TraceStateTest, Entries) opentelemetry::nostd::span entries = s.Entries(); for (int i = 0; i < kNumPairs; i++) { - EXPECT_EQ(entries[i].GetKey().data(), keys[i]); - EXPECT_EQ(entries[i].GetValue().data(), values[i]); + EXPECT_EQ(entries[i].GetKey(), keys[i]); + EXPECT_EQ(entries[i].GetValue(), values[i]); } } From 47d5a55c9d79c21febf62c3a7e837394df3f59aa Mon Sep 17 00:00:00 2001 From: Hudson Humphries Date: Wed, 19 Aug 2020 16:22:19 -0500 Subject: [PATCH 2/5] Use Struct as Key in Processor (#272) --- .../sdk/metrics/ungrouped_processor.h | 43 ++++++++++++++++++- sdk/src/metrics/ungrouped_processor.cc | 27 ++---------- 2 files changed, 46 insertions(+), 24 deletions(-) diff --git a/sdk/include/opentelemetry/sdk/metrics/ungrouped_processor.h b/sdk/include/opentelemetry/sdk/metrics/ungrouped_processor.h index 4becf57e6a..f0449be0ba 100644 --- a/sdk/include/opentelemetry/sdk/metrics/ungrouped_processor.h +++ b/sdk/include/opentelemetry/sdk/metrics/ungrouped_processor.h @@ -18,6 +18,47 @@ namespace sdk namespace metrics { + +struct KeyStruct +{ + std::string name; + std::string description; + std::string labels; + metrics_api::InstrumentKind ins_kind; + + // constructor + KeyStruct(std::string name, + std::string description, + std::string labels, + metrics_api::InstrumentKind ins_kind) + { + this->name = name; + this->description = description; + this->labels = labels; + this->ins_kind = ins_kind; + } + + // operator== is required to compare keys in case of hash collision + bool operator==(const KeyStruct &p) const + { + return name == p.name && description == p.description && labels == p.labels && + ins_kind == p.ins_kind; + } +}; + +struct KeyStruct_Hash +{ + std::size_t operator()(const KeyStruct &keystruct) const + { + std::size_t name_size = keystruct.name.length(); + std::size_t desc_size = keystruct.description.length(); + std::size_t labels_size = keystruct.labels.length(); + std::size_t ins_size = (int)keystruct.ins_kind; + + return (name_size ^ desc_size ^ labels_size) + ins_size; + } +}; + class UngroupedMetricsProcessor : public MetricsProcessor { public: @@ -31,7 +72,7 @@ class UngroupedMetricsProcessor : public MetricsProcessor private: bool stateful_; - std::unordered_map batch_map_; + std::unordered_map batch_map_; /** * get_instrument returns the instrument from the passed in AggregatorVariant. We have to diff --git a/sdk/src/metrics/ungrouped_processor.cc b/sdk/src/metrics/ungrouped_processor.cc index ab1e8837fa..4c5be0e220 100644 --- a/sdk/src/metrics/ungrouped_processor.cc +++ b/sdk/src/metrics/ungrouped_processor.cc @@ -1,7 +1,5 @@ #include "opentelemetry/sdk/metrics/ungrouped_processor.h" -#define UNGROUPED_PROCESSOR_STRINGER(x) (#x) - OPENTELEMETRY_BEGIN_NAMESPACE namespace sdk @@ -25,22 +23,9 @@ std::vector UngroupedMetricsProcessor::CheckpointSelf() noex for (auto iter : batch_map_) { - /** - * TODO: micro-optimization, scan once or change to using a struct with custom hash function, - * to hold the data. - */ - - std::string key = iter.first; - std::size_t description_index = key.find("/description/"); - std::size_t labels_index = key.find("/labels/"); - std::size_t instrument_index = key.find("/instrument/"); - - std::string name = key.substr(6, description_index - 6); - std::string description = - key.substr(description_index + 13, labels_index - description_index - 13); - std::string labels = key.substr(labels_index + 8, instrument_index - labels_index - 8); - - sdkmetrics::Record r{name, description, labels, iter.second}; + // Create a record from the held KeyStruct values and add to the Checkpoint + KeyStruct key = iter.first; + sdkmetrics::Record r{key.name, key.description, key.labels, iter.second}; metric_records.push_back(r); } @@ -66,10 +51,8 @@ void UngroupedMetricsProcessor::process(sdkmetrics::Record record) noexcept std::string label = record.GetLabels(); std::string name = record.GetName(); std::string description = record.GetDescription(); - std::string instrument = UNGROUPED_PROCESSOR_STRINGER(get_instrument(aggregator)); - std::string batch_key = "/name/" + name + "/description/" + description + "/labels/" + label + - "/instrument/" + instrument; + KeyStruct batch_key = KeyStruct(name, description, label, get_instrument(aggregator)); /** * If we have already seen this aggregator then we will merge it with the copy that exists in the @@ -178,5 +161,3 @@ void UngroupedMetricsProcessor::process(sdkmetrics::Record record) noexcept } // namespace sdk OPENTELEMETRY_END_NAMESPACE - -#undef UNGROUPED_PROCESSOR_STRINGER From c0c4fb355eba85a9646851c4a697e427a3981ef1 Mon Sep 17 00:00:00 2001 From: Keshav Manghat Date: Wed, 19 Aug 2020 15:50:24 -0600 Subject: [PATCH 3/5] zPages: zPages tracez wrapper (#267) --- ext/include/opentelemetry/ext/zpages/zpages.h | 65 +++++++++++++++++++ 1 file changed, 65 insertions(+) create mode 100644 ext/include/opentelemetry/ext/zpages/zpages.h diff --git a/ext/include/opentelemetry/ext/zpages/zpages.h b/ext/include/opentelemetry/ext/zpages/zpages.h new file mode 100644 index 0000000000..9ef7ea5412 --- /dev/null +++ b/ext/include/opentelemetry/ext/zpages/zpages.h @@ -0,0 +1,65 @@ +#pragma once + +#include +#include + +#include "opentelemetry/ext/zpages/tracez_data_aggregator.h" +#include "opentelemetry/ext/zpages/tracez_http_server.h" +#include "opentelemetry/ext/zpages/tracez_processor.h" + +#include "opentelemetry/nostd/shared_ptr.h" +#include "opentelemetry/sdk/trace/tracer_provider.h" +#include "opentelemetry/trace/provider.h" + +using opentelemetry::ext::zpages::TracezDataAggregator; +using opentelemetry::ext::zpages::TracezHttpServer; +using opentelemetry::ext::zpages::TracezSpanProcessor; +using std::chrono::microseconds; + +/** + * Wrapper for zPages that initializes all the components required for zPages, + * and starts the HTTP server in the constructor and ends it in the destructor. + * The constructor and destructor for this object is private to prevent + * creation other than by calling the static function Initialize(). This follows the + * meyers singleton pattern and only a single instance of the class is allowed. + */ +class ZPages +{ +public: + /** + * This function is called if the user wishes to include zPages in their + * application. It creates a static instance of this class. + */ + static void Initialize() { static ZPages instance; } + +private: + /** + * Constructor is responsible for initializing the tracer, tracez processor, + * tracez data aggregator and the tracez server. The server is also started in + * constructor. + */ + ZPages() + { + auto tracez_processor_ = std::make_shared(); + auto tracez_provider_ = opentelemetry::nostd::shared_ptr( + new opentelemetry::sdk::trace::TracerProvider(tracez_processor_)); + + auto tracez_aggregator = + std::unique_ptr(new TracezDataAggregator(tracez_processor_)); + + tracez_server_ = + std::unique_ptr(new TracezHttpServer(std::move(tracez_aggregator))); + + tracez_server_->start(); + + opentelemetry::trace::Provider::SetTracerProvider(tracez_provider_); + } + + ~ZPages() + { + // shut down the server when the object goes out of scope(at the end of the + // program) + tracez_server_->stop(); + } + std::unique_ptr tracez_server_; +}; From 021bd5e7a2fbc68216267ba7a8f0916a22402b44 Mon Sep 17 00:00:00 2001 From: Keshav Manghat Date: Wed, 19 Aug 2020 21:19:14 -0600 Subject: [PATCH 4/5] zPages: zPages tracez basic example (#268) --- examples/zpages/BUILD | 31 +++++++++++++++++ examples/zpages/zpages_example.cc | 57 +++++++++++++++++++++++++++++++ 2 files changed, 88 insertions(+) create mode 100644 examples/zpages/BUILD create mode 100644 examples/zpages/zpages_example.cc diff --git a/examples/zpages/BUILD b/examples/zpages/BUILD new file mode 100644 index 0000000000..a277c44810 --- /dev/null +++ b/examples/zpages/BUILD @@ -0,0 +1,31 @@ +# 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_binary( + name = "zpages_example", + srcs = [ + "zpages_example.cc", + ], + linkopts = select({ + "//bazel:windows": [], + "//conditions:default": ["-pthread"], + }), + deps = [ + "//ext:headers", + "//ext/src/zpages", + "//sdk/src/trace", + ], +) diff --git a/examples/zpages/zpages_example.cc b/examples/zpages/zpages_example.cc new file mode 100644 index 0000000000..f89f8f95d9 --- /dev/null +++ b/examples/zpages/zpages_example.cc @@ -0,0 +1,57 @@ +/** + * This is a basic example for zpages that helps users get familiar with how to + * use this feature in OpenTelemetery + */ +#include +#include +#include +#include "opentelemetry/context/threadlocal_context.h" + +#include "opentelemetry/ext/zpages/zpages.h" // Required file include for zpages + +using opentelemetry::core::SteadyTimestamp; + +int main(int argc, char *argv[]) +{ + + /** + * The following line initializes zPages and starts a webserver at + * http://localhost:30000/tracez/ where spans that are created in the application + * can be viewed. + * Note that the webserver is destroyed after the application ends execution. + */ + ZPages::Initialize(); + auto tracer = opentelemetry::trace::Provider::GetTracerProvider()->GetTracer(""); + + std::cout << "This example for zPages creates a few types of spans and then " + << "creates a span every second for the duration of the application" + << "\n"; + + // Error span + std::map attribute_map; + attribute_map["completed_search_for"] = "Unknown user"; + tracer->StartSpan("find user", attribute_map) + ->SetStatus(opentelemetry::trace::CanonicalCode::NOT_FOUND, "User not found"); + + // Long time duration span + std::map attribute_map2; + attribute_map2["completed_search_for"] = "John Doe"; + opentelemetry::trace::StartSpanOptions start; + start.start_steady_time = SteadyTimestamp(nanoseconds(1)); + opentelemetry::trace::EndSpanOptions end; + end.end_steady_time = SteadyTimestamp(nanoseconds(1000000000000)); + tracer->StartSpan("find user", attribute_map2, start)->End(end); + + // Running(deadlock) span + std::map attribute_map3; + attribute_map3["searching_for"] = "Deleted user"; + auto running_span = tracer->StartSpan("find user", attribute_map3); + + // Create a completed span every second till user stops the loop + std::cout << "Presss CTRL+C to stop...\n"; + while (true) + { + std::this_thread::sleep_for(seconds(1)); + tracer->StartSpan("ping user")->End(); + } +} From 1ce011f1763f6a357eb01a6d95466fd9eece902f Mon Sep 17 00:00:00 2001 From: Eric Hsueh <39718333+erichsueh3@users.noreply.github.com> Date: Thu, 20 Aug 2020 19:19:58 -0700 Subject: [PATCH 5/5] Add Prometheus Exporter: Step 0 - Update MetricsExporter Interface (#240) --- .../opentelemetry/sdk/metrics/exporter.h | 34 +++++++++++++++---- 1 file changed, 27 insertions(+), 7 deletions(-) diff --git a/sdk/include/opentelemetry/sdk/metrics/exporter.h b/sdk/include/opentelemetry/sdk/metrics/exporter.h index d1708e56c1..5a66f11815 100644 --- a/sdk/include/opentelemetry/sdk/metrics/exporter.h +++ b/sdk/include/opentelemetry/sdk/metrics/exporter.h @@ -1,3 +1,19 @@ +/* + * Copyright The 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. + */ + #pragma once #include @@ -22,7 +38,17 @@ enum class ExportResult * Exporting failed. The caller must not retry exporting the same batch; the * batch must be dropped. */ - kFailure + kFailure = 1, + + /** + * The collection does not have enough space to receive the export batch. + */ + kFailureFull = 2, + + /** + * The export() function was passed an invalid argument. + */ + kFailureInvalidArgument = 3, }; /** * MetricsExporter defines the interface that protocol-specific span exporters must @@ -39,12 +65,6 @@ class MetricsExporter * @param records a vector of unique pointers to metric records */ virtual ExportResult Export(const std::vector &records) noexcept = 0; - - /** - * In the Metrics specification, there is no Shutdown function required for exporters - * The Shutdown function can be implemented within exporters that wish to have one, - * but will not be enforced through this header - */ }; } // namespace metrics } // namespace sdk