From 7687460d1c054938bc2d2bf4db7aca10d9cb7643 Mon Sep 17 00:00:00 2001 From: Ryan Burn Date: Fri, 9 Jun 2017 19:55:33 -0400 Subject: [PATCH 01/42] Set up cpack and version number. --- CMakeLists.txt | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/CMakeLists.txt b/CMakeLists.txt index 2f16e23..a3e330c 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -3,4 +3,14 @@ enable_testing() project(opentracing-cpp) +SET(CPACK_PACKAGE_DESCRIPTION_SUMMARY "C++ implementation of the OpenTracing API") +SET(CPACK_PACKAGE_VENDOR "opentracing.io") +SET(CPACK_PACKAGE_DESCRIPTION_FILE "${CMAKE_CURRENT_SOURCE_DIR}/README.md") +SET(CPACK_RESOURCE_FILE_LICENSE "${CMAKE_CURRENT_SOURCE_DIR}/LICENSE") + +SET(CPACK_PACKAGE_VERSION_MAJOR "1") +SET(CPACK_PACKAGE_VERSION_MINOR "0") +SET(CPACK_PACKAGE_VERSION_PATCH "0") +include(cpack) + add_subdirectory(c++11) From 9e83d77d1566c73163e83ed366ccf18a78bdf97c Mon Sep 17 00:00:00 2001 From: Ryan Burn Date: Fri, 9 Jun 2017 21:37:27 -0400 Subject: [PATCH 02/42] Make NoopSpan constructor explicit. --- c++11/src/noop.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/c++11/src/noop.cpp b/c++11/src/noop.cpp index 4bc1b31..7bbe2b3 100644 --- a/c++11/src/noop.cpp +++ b/c++11/src/noop.cpp @@ -12,7 +12,7 @@ class NoopSpanContext : public SpanContext { class NoopSpan : public Span { public: - NoopSpan(std::shared_ptr&& tracer) + explicit NoopSpan(std::shared_ptr&& tracer) : tracer_(std::move(tracer)) {} void FinishWithOptions( const FinishSpanOptions& finish_span_options) noexcept override {} From 7585a62566f92ec9e976d4b2653f505e11ad0df8 Mon Sep 17 00:00:00 2001 From: Ryan Burn Date: Fri, 9 Jun 2017 23:09:44 -0400 Subject: [PATCH 03/42] Add additional noexcept specifiers. --- c++11/include/opentracing/preprocessor.h | 1 + c++11/include/opentracing/span.h | 11 ++++---- c++11/include/opentracing/stringref.h | 4 +-- c++11/include/opentracing/tracer.h | 36 ++++++++++++++---------- c++11/src/noop.cpp | 18 ++++++------ c++11/test/noop_test.cpp | 2 +- c++11/test/stringref_test.cpp | 4 --- 7 files changed, 41 insertions(+), 35 deletions(-) diff --git a/c++11/include/opentracing/preprocessor.h b/c++11/include/opentracing/preprocessor.h index 0634e82..be4cfdc 100644 --- a/c++11/include/opentracing/preprocessor.h +++ b/c++11/include/opentracing/preprocessor.h @@ -1,6 +1,7 @@ #ifndef OPENTRACING_PREPROCESSOR_H #define OPENTRACING_PREPROCESSOR_H +// Increase when an ABI-breaking change is made. #define OPENTRACING_VERSION_NAMESPACE v1 #endif // OPENTRACING_PREPROCESSOR_H diff --git a/c++11/include/opentracing/span.h b/c++11/include/opentracing/span.h index c656e60..9ab1738 100644 --- a/c++11/include/opentracing/span.h +++ b/c++11/include/opentracing/span.h @@ -71,7 +71,7 @@ class Span { // // If SetOperationName is called after Finish it leaves the Span in a valid // state, but its behavior is unspecified. - virtual void SetOperationName(StringRef name) = 0; + virtual void SetOperationName(StringRef name) noexcept = 0; // Adds a tag to the span. // @@ -84,7 +84,7 @@ class Span { // // If SetTag is called after Finish it leaves the Span in a valid state, but // its behavior is unspecified. - virtual void SetTag(StringRef key, const Value& value) = 0; + virtual void SetTag(StringRef key, const Value& value) noexcept = 0; // SetBaggageItem sets a key:value pair on this Span and its SpanContext // that also propagates to descendants of this Span. @@ -103,11 +103,12 @@ class Span { // // If SetBaggageItem is called after Finish it leaves the Span in a valid // state, but its behavior is unspecified. - virtual void SetBaggageItem(StringRef restricted_key, StringRef value) = 0; + virtual void SetBaggageItem(StringRef restricted_key, + StringRef value) noexcept = 0; // Gets the value for a baggage item given its key. Returns the empty string // if the value isn't found in this Span. - virtual std::string BaggageItem(StringRef restricted_key) const = 0; + virtual StringRef BaggageItem(StringRef restricted_key) const noexcept = 0; // context() yields the SpanContext for this Span. Note that the return // value of context() is still valid after a call to Span.Finish(), as is @@ -127,7 +128,7 @@ class FinishTimestamp : public FinishSpanOption { template explicit FinishTimestamp( - const std::chrono::duration& time_since_epoch) + const std::chrono::duration& time_since_epoch) noexcept : steady_when_(time_since_epoch) {} FinishTimestamp(const FinishTimestamp& other) noexcept diff --git a/c++11/include/opentracing/stringref.h b/c++11/include/opentracing/stringref.h index ae47ac1..d7aca6b 100644 --- a/c++11/include/opentracing/stringref.h +++ b/c++11/include/opentracing/stringref.h @@ -71,8 +71,8 @@ class StringRef { template StringRef(char (&str)[N]) = delete; - // Implicit conversion to plain char * - operator const char*() const noexcept { return data_; } + // Implicit conversion to std::string + operator std::string() const { return {data_, length_}; } // Reset the string reference given a const character array template diff --git a/c++11/include/opentracing/tracer.h b/c++11/include/opentracing/tracer.h index c0ae8df..31249b4 100644 --- a/c++11/include/opentracing/tracer.h +++ b/c++11/include/opentracing/tracer.h @@ -33,7 +33,7 @@ class StartSpanOption { virtual ~StartSpanOption() = default; - virtual void Apply(StartSpanOptions& options) const = 0; + virtual void Apply(StartSpanOptions& options) const noexcept = 0; protected: StartSpanOption() = default; @@ -73,7 +73,7 @@ class Tracer { std::unique_ptr StartSpan( StringRef operation_name, std::initializer_list> option_list = {}) - const { + const noexcept { StartSpanOptions options; options.start_system_timestamp = SystemClock::now(); options.start_steady_timestamp = SteadyClock::now(); @@ -82,7 +82,8 @@ class Tracer { } virtual std::unique_ptr StartSpanWithOptions( - StringRef operation_name, const StartSpanOptions& options) const = 0; + StringRef operation_name, const StartSpanOptions& options) const + noexcept = 0; // Inject() takes the `sc` SpanContext instance and injects it for // propagation within `carrier`. The actual type of `carrier` depends on @@ -116,20 +117,20 @@ class Tracer { // the new Span. class StartTimestamp : public StartSpanOption { public: - StartTimestamp(SystemTime system_when, SteadyTime steady_when) + StartTimestamp(SystemTime system_when, SteadyTime steady_when) noexcept : system_when_(system_when), steady_when_(steady_when) {} template explicit StartTimestamp( - const std::chrono::duration& time_since_epoch) + const std::chrono::duration& time_since_epoch) noexcept : system_when_(time_since_epoch), steady_when_(time_since_epoch) {} - StartTimestamp(const StartTimestamp& other) + StartTimestamp(const StartTimestamp& other) noexcept : StartSpanOption(), system_when_(other.system_when_), steady_when_(other.steady_when_) {} - void Apply(StartSpanOptions& options) const override { + void Apply(StartSpanOptions& options) const noexcept override { options.start_system_timestamp = system_when_; options.start_steady_timestamp = steady_when_; } @@ -144,14 +145,16 @@ class StartTimestamp : public StartSpanOption { // supported relationships. class SpanReference : public StartSpanOption { public: - SpanReference(SpanReferenceType type, const SpanContext& referenced) + SpanReference(SpanReferenceType type, const SpanContext& referenced) noexcept : type_(type), referenced_(referenced) {} - SpanReference(const SpanReference& other) + SpanReference(const SpanReference& other) noexcept : StartSpanOption(), type_(other.type_), referenced_(other.referenced_) {} - void Apply(StartSpanOptions& options) const override { + void Apply(StartSpanOptions& options) const noexcept override try { options.references.emplace_back(type_, &referenced_); + } catch (const std::bad_alloc&) { + // Ignore reference if memory can't be allocated for it. } private: @@ -162,7 +165,7 @@ class SpanReference : public StartSpanOption { // ChildOf returns a StartSpanOption pointing to a dependent parent span. // // See ChildOfRef, SpanReference -inline SpanReference ChildOf(const SpanContext& span_context) { +inline SpanReference ChildOf(const SpanContext& span_context) noexcept { return {SpanReferenceType::ChildOfRef, span_context}; } @@ -170,7 +173,7 @@ inline SpanReference ChildOf(const SpanContext& span_context) { // the child Span but does not directly depend on its result in any way. // // See FollowsFromRef, SpanReference -inline SpanReference FollowsFrom(const SpanContext& span_context) { +inline SpanReference FollowsFrom(const SpanContext& span_context) noexcept { return {SpanReferenceType::FollowsFromRef, span_context}; } @@ -180,13 +183,16 @@ inline SpanReference FollowsFrom(const SpanContext& span_context) { // tracer.StartSpan("opName", SetTag{"Key", value}) class SetTag : public StartSpanOption { public: - SetTag(StringRef key, const Value& value) : key_(key), value_(value) {} + SetTag(StringRef key, const Value& value) noexcept + : key_(key), value_(value) {} - SetTag(const SetTag& other) + SetTag(const SetTag& other) noexcept : StartSpanOption(), key_(other.key_), value_(other.value_) {} - void Apply(StartSpanOptions& options) const override { + void Apply(StartSpanOptions& options) const noexcept override try { options.tags.emplace_back(key_, value_); + } catch (const std::bad_alloc&) { + // Ignore tag if memory can't be allocated for it. } private: diff --git a/c++11/src/noop.cpp b/c++11/src/noop.cpp index 7bbe2b3..6c63eaa 100644 --- a/c++11/src/noop.cpp +++ b/c++11/src/noop.cpp @@ -12,14 +12,15 @@ class NoopSpanContext : public SpanContext { class NoopSpan : public Span { public: - explicit NoopSpan(std::shared_ptr&& tracer) + explicit NoopSpan(std::shared_ptr&& tracer) noexcept : tracer_(std::move(tracer)) {} void FinishWithOptions( const FinishSpanOptions& finish_span_options) noexcept override {} - void SetOperationName(StringRef name) override {} - void SetTag(StringRef key, const Value& value) override {} - void SetBaggageItem(StringRef restricted_key, StringRef value) override {} - std::string BaggageItem(StringRef restricted_key) const override { + void SetOperationName(StringRef name) noexcept override {} + void SetTag(StringRef key, const Value& value) noexcept override {} + void SetBaggageItem(StringRef restricted_key, + StringRef value) noexcept override {} + StringRef BaggageItem(StringRef restricted_key) const noexcept override { return {}; } const SpanContext& context() const noexcept override { return span_context_; } @@ -34,9 +35,10 @@ class NoopTracer : public Tracer, public std::enable_shared_from_this { public: std::unique_ptr StartSpanWithOptions( - StringRef operation_name, - const StartSpanOptions& options) const override { - return std::unique_ptr(new NoopSpan(shared_from_this())); + StringRef operation_name, const StartSpanOptions& options) const + noexcept override { + return std::unique_ptr(new (std::nothrow) + NoopSpan(shared_from_this())); } Expected Inject( diff --git a/c++11/test/noop_test.cpp b/c++11/test/noop_test.cpp index 9771328..5c3313b 100644 --- a/c++11/test/noop_test.cpp +++ b/c++11/test/noop_test.cpp @@ -12,7 +12,7 @@ int main() { auto span2 = tracer->StartSpan("b", {ChildOf(span1->context())}); span2->SetOperationName("b1"); span2->SetTag("x", true); - assert(span2->BaggageItem("y") == ""); + assert(span2->BaggageItem("y").data() == nullptr); span2->Finish(); return 0; diff --git a/c++11/test/stringref_test.cpp b/c++11/test/stringref_test.cpp index fb97da4..8bd92e5 100644 --- a/c++11/test/stringref_test.cpp +++ b/c++11/test/stringref_test.cpp @@ -61,7 +61,6 @@ static void test_reset() { const char arr[] = "hello world 1"; ref.reset(arr); - assert(arr == ref); assert(arr == ref.data()); assert(std::strlen(arr) == ref.length()); assert(std::string(arr) == std::string(ref)); @@ -70,7 +69,6 @@ static void test_reset() { const char* p = "hello world 2"; ref.reset(p); - assert(p == ref); assert(p == ref.data()); assert(std::strlen(p) == ref.length()); assert(std::string(p) == std::string(ref)); @@ -79,14 +77,12 @@ static void test_reset() { const std::string s = "hello world 3"; ref.reset(s); - assert(s.data() == ref); assert(s.data() == ref.data()); assert(s.length() == ref.length()); assert(std::string(s.c_str()) == std::string(ref)); assert(std::string(s.c_str()) == std::string(ref.data())); ref.reset(p, std::strlen(p)); - assert(p == ref); assert(p == ref.data()); assert(std::strlen(p) == ref.length()); assert(std::string(p) == std::string(ref)); From 026c214b84375455fbe5a0bce25c7d37ad946d74 Mon Sep 17 00:00:00 2001 From: Ryan Burn Date: Fri, 9 Jun 2017 23:25:41 -0400 Subject: [PATCH 04/42] Change return type for BaggageItem. --- c++11/include/opentracing/span.h | 2 +- c++11/src/noop.cpp | 2 +- c++11/test/noop_test.cpp | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/c++11/include/opentracing/span.h b/c++11/include/opentracing/span.h index 9ab1738..b225a27 100644 --- a/c++11/include/opentracing/span.h +++ b/c++11/include/opentracing/span.h @@ -108,7 +108,7 @@ class Span { // Gets the value for a baggage item given its key. Returns the empty string // if the value isn't found in this Span. - virtual StringRef BaggageItem(StringRef restricted_key) const noexcept = 0; + virtual std::string BaggageItem(StringRef restricted_key) const noexcept = 0; // context() yields the SpanContext for this Span. Note that the return // value of context() is still valid after a call to Span.Finish(), as is diff --git a/c++11/src/noop.cpp b/c++11/src/noop.cpp index 6c63eaa..62df584 100644 --- a/c++11/src/noop.cpp +++ b/c++11/src/noop.cpp @@ -20,7 +20,7 @@ class NoopSpan : public Span { void SetTag(StringRef key, const Value& value) noexcept override {} void SetBaggageItem(StringRef restricted_key, StringRef value) noexcept override {} - StringRef BaggageItem(StringRef restricted_key) const noexcept override { + std::string BaggageItem(StringRef restricted_key) const noexcept override { return {}; } const SpanContext& context() const noexcept override { return span_context_; } diff --git a/c++11/test/noop_test.cpp b/c++11/test/noop_test.cpp index 5c3313b..56d1f2f 100644 --- a/c++11/test/noop_test.cpp +++ b/c++11/test/noop_test.cpp @@ -12,7 +12,7 @@ int main() { auto span2 = tracer->StartSpan("b", {ChildOf(span1->context())}); span2->SetOperationName("b1"); span2->SetTag("x", true); - assert(span2->BaggageItem("y").data() == nullptr); + assert(span2->BaggageItem("y").empty()); span2->Finish(); return 0; From fe3f89f8d124f861c44e5a7e1288bcf71ed70bfb Mon Sep 17 00:00:00 2001 From: Ryan Burn Date: Sat, 10 Jun 2017 15:01:09 -0400 Subject: [PATCH 05/42] Don't include StringRef as a Value type. --- c++11/include/opentracing/value.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/c++11/include/opentracing/value.h b/c++11/include/opentracing/value.h index 9391bc8..27e7bc8 100644 --- a/c++11/include/opentracing/value.h +++ b/c++11/include/opentracing/value.h @@ -17,7 +17,7 @@ class Value; typedef std::unordered_map Dictionary; typedef std::vector Values; typedef mapbox::util::variant, mapbox::util::recursive_wrapper> variant_type; From 49318d5722e6a9c215c736289525941749547561 Mon Sep 17 00:00:00 2001 From: Ryan Burn Date: Sun, 11 Jun 2017 16:12:25 -0400 Subject: [PATCH 06/42] Cast durations when setting timestamps from epoch. --- c++11/include/opentracing/tracer.h | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/c++11/include/opentracing/tracer.h b/c++11/include/opentracing/tracer.h index 31249b4..df330ae 100644 --- a/c++11/include/opentracing/tracer.h +++ b/c++11/include/opentracing/tracer.h @@ -6,6 +6,7 @@ #include #include #include +#include #include #include #include @@ -123,7 +124,10 @@ class StartTimestamp : public StartSpanOption { template explicit StartTimestamp( const std::chrono::duration& time_since_epoch) noexcept - : system_when_(time_since_epoch), steady_when_(time_since_epoch) {} + : system_when_(std::chrono::duration_cast( + time_since_epoch)), + steady_when_(std::chrono::duration_cast( + time_since_epoch)) {} StartTimestamp(const StartTimestamp& other) noexcept : StartSpanOption(), From 19ee9f1bea0eeae1142dd8902e0e05ec7a13f77f Mon Sep 17 00:00:00 2001 From: Ryan Burn Date: Mon, 12 Jun 2017 11:50:37 -0400 Subject: [PATCH 07/42] Fix namespace issue. --- c++11/include/opentracing/stringref.h | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/c++11/include/opentracing/stringref.h b/c++11/include/opentracing/stringref.h index d7aca6b..8214123 100644 --- a/c++11/include/opentracing/stringref.h +++ b/c++11/include/opentracing/stringref.h @@ -1,6 +1,11 @@ #ifndef OPENTRACING_STRINGREF_H #define OPENTRACING_STRINGREF_H +#include +#include +#include +#include + // =========== // stringref.h // =========== @@ -30,10 +35,6 @@ // that breaks if it was expecting wstring and starts receiving string all of a // sudden. That design issue still needs to be addressed. -#include -#include -#include - namespace opentracing { inline namespace OPENTRACING_VERSION_NAMESPACE { // =============== From 67deb6411ec5fd119fa66543669cfce86ca68e87 Mon Sep 17 00:00:00 2001 From: Ryan Burn Date: Mon, 12 Jun 2017 19:13:24 -0400 Subject: [PATCH 08/42] Rewrite constructors for Value type. --- c++11/include/opentracing/value.h | 35 ++++++++++++++++++++++++++++-- c++11/test/CMakeLists.txt | 3 +++ c++11/test/value_test.cpp | 36 +++++++++++++++++++++++++++++++ 3 files changed, 72 insertions(+), 2 deletions(-) create mode 100644 c++11/test/value_test.cpp diff --git a/c++11/include/opentracing/value.h b/c++11/include/opentracing/value.h index 27e7bc8..afbf19b 100644 --- a/c++11/include/opentracing/value.h +++ b/c++11/include/opentracing/value.h @@ -25,12 +25,43 @@ typedef mapbox::util::variant - Value(T&& t) : variant_type(std::forward(t)) {} + // variant_type's constructors will do some undesirable casting, for example + // variant_type(123) + // will construct a bool variant; hence, constructors are expanded + // out so as to provide more sensible behavior. + Value(bool x) : variant_type(x) {} + + template ::value && + std::is_signed::value>::type* = nullptr> + Value(T t) : variant_type(static_cast(t)) {} + + template < + typename T, + typename std::enable_if::value && + std::is_unsigned::value>::type* = nullptr> + Value(T t) : variant_type(static_cast(t)) {} + + template ::value>::type* = + nullptr> + Value(T t) : variant_type(static_cast(t)) {} + + Value(const char* s) : variant_type(s) {} template Value(const char (&cstr)[N]) : variant_type(std::string(cstr)) {} + + Value(const std::string& s) : variant_type(s) {} + Value(std::string&& s) : variant_type(std::move(s)) {} + + Value(const Values& values) : variant_type(values) {} + Value(Values&& values) : variant_type(std::move(values)) {} + + Value(const Dictionary& values) : variant_type(values) {} + Value(Dictionary&& values) : variant_type(std::move(values)) {} }; } // namespace OPENTRACING_VERSION_NAMESPACE } // namespace opentracing diff --git a/c++11/test/CMakeLists.txt b/c++11/test/CMakeLists.txt index cdf3026..b37379a 100644 --- a/c++11/test/CMakeLists.txt +++ b/c++11/test/CMakeLists.txt @@ -4,3 +4,6 @@ add_test(noop_test noop_test) add_executable(stringref_test stringref_test.cpp) add_test(stringref_test stringref_test) + +add_executable(value_test value_test.cpp) +add_test(value_test value_test) diff --git a/c++11/test/value_test.cpp b/c++11/test/value_test.cpp new file mode 100644 index 0000000..a204393 --- /dev/null +++ b/c++11/test/value_test.cpp @@ -0,0 +1,36 @@ +#include +#include +using namespace opentracing; + +static void test_implicit_construction() { + Value v1(123); + assert(v1.is()); + + Value v2(123u); + assert(v2.is()); + + Value v3(true); + assert(v3.is()); + + Value v4(1.0); + assert(v4.is()); + Value v5(1.0f); + assert(v5.is()); + + Value v6(std::string("abc")); + assert(v6.is()); + + Value v7("abc"); + assert(v7.is()); + + Value v8(Values{Value(1), Value(2)}); + (void)v8; + + Value v9(Dictionary{{"abc", Value(123)}}); + (void)v9; +} + +int main() { + test_implicit_construction(); + return 0; +} From 57b1da842be62faa42b55ab80ff498b018e9578e Mon Sep 17 00:00:00 2001 From: Ryan Burn Date: Tue, 13 Jun 2017 14:10:58 -0400 Subject: [PATCH 09/42] Rewrote the error handling interface. --- c++11/CMakeLists.txt | 3 +- c++11/include/opentracing/propagation.h | 44 ++++++++++++++++++++-- c++11/include/opentracing/tracer.h | 7 ++-- c++11/include/opentracing/util.h | 8 +++- c++11/src/noop.cpp | 9 ++--- c++11/src/propagation.cpp | 50 +++++++++++++++++++++++++ 6 files changed, 107 insertions(+), 14 deletions(-) create mode 100644 c++11/src/propagation.cpp diff --git a/c++11/CMakeLists.txt b/c++11/CMakeLists.txt index 6c00afe..d8bf9cd 100644 --- a/c++11/CMakeLists.txt +++ b/c++11/CMakeLists.txt @@ -6,6 +6,7 @@ if ("${CMAKE_CXX_COMPILER_ID}" STREQUAL "Clang") -Wno-unused-parameter \ -Wno-weak-vtables \ -Wno-exit-time-destructors \ + -Wno-global-constructors \ -Wno-padded") elseif ("${CMAKE_CXX_COMPILER_ID}" STREQUAL "GNU") set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -Wall -Wextra -Wno-unused-parameter") @@ -20,7 +21,7 @@ include_directories(include) install(DIRECTORY include/opentracing DESTINATION include FILES_MATCHING PATTERN "*.h") -set(SRCS src/noop.cpp src/tracer.cpp) +set(SRCS src/propagation.cpp src/noop.cpp src/tracer.cpp) add_library(opentracing SHARED ${SRCS}) add_library(opentracing-static STATIC ${SRCS}) set_target_properties(opentracing-static PROPERTIES OUTPUT_NAME opentracing) diff --git a/c++11/include/opentracing/propagation.h b/c++11/include/opentracing/propagation.h index d7d2b44..b8d3423 100644 --- a/c++11/include/opentracing/propagation.h +++ b/c++11/include/opentracing/propagation.h @@ -6,6 +6,7 @@ #include #include #include +#include namespace opentracing { inline namespace OPENTRACING_VERSION_NAMESPACE { @@ -98,7 +99,44 @@ enum class CarrierFormat { TextMap = 3 }; -// Base class for implementation-dependent Tracer::Inject carrier-type adapter. +// Returns the std::error_category class used for opentracing propagation +// errors. +// +// See +// http://blog.think-async.com/2010/04/system-error-support-in-c0x-part-1.html +// https://ned14.github.io/boost.outcome/md_doc_md_03-tutorial_b.html +const std::error_category& propagation_error_category(); + +// `unsupported_format_error`` occurs when the `format` passed to +// Tracer::Inject() or Tracer::Extract() is not recognized by the Tracer +// implementation. +const std::error_code unsupported_format_error(0, propagation_error_category()); + +// `span_context_not_found_error` occurs when the `carrier` passed to +// Tracer::Extract() is valid and uncorrupted but has insufficient +// information to extract a SpanContext. +const std::error_code span_context_not_found_error( + 1, propagation_error_category()); + +// `invalid_span_context_error` errors occur when Tracer::Inject() is asked to +// operate on a SpanContext which it is not prepared to handle (for +// example, since it was created by a different tracer implementation). +const std::error_code invalid_span_context_error(2, + propagation_error_category()); + +// `invalid_carrier_error` errors occur when Tracer::Inject() or +// Tracer::Extract() +// implementations expect a different type of `carrier` than they are +// given. +const std::error_code invalid_carrier_error(3, propagation_error_category()); + +// `span_context_corrupted_error` occurs when the `carrier` passed to +// Tracer::Extract() is of the expected type but is corrupted. +const std::error_code span_context_corrupted_error( + 4, propagation_error_category()); + +// Base class for implementation-dependent Tracer::Inject carrier-type +// adapter. class CarrierReader { public: virtual ~CarrierReader() = default; @@ -120,8 +158,8 @@ class CarrierWriter { // Basic foundation for OpenTracing basictracer-compatible carrier writers. class BasicCarrierWriter : public CarrierWriter { public: - virtual Expected Set(const std::string& key, - const std::string& value) const = 0; + virtual Expected Set(const std::string& key, + const std::string& value) const = 0; }; // Base class for injecting into TextMap and HTTPHeaders carriers. diff --git a/c++11/include/opentracing/tracer.h b/c++11/include/opentracing/tracer.h index df330ae..6497c0e 100644 --- a/c++11/include/opentracing/tracer.h +++ b/c++11/include/opentracing/tracer.h @@ -92,9 +92,8 @@ class Tracer { // // OpenTracing defines a common set of `format` values (see BuiltinFormat), // and each has an expected carrier type. - virtual Expected Inject( - const SpanContext& sc, CarrierFormat format, - const CarrierWriter& writer) const = 0; + virtual Expected Inject(const SpanContext& sc, CarrierFormat format, + const CarrierWriter& writer) const = 0; // Extract() returns a SpanContext instance given `format` and `carrier`. // @@ -102,7 +101,7 @@ class Tracer { // and each has an expected carrier type. // // Returns a `SpanContext` that is `non-null` on success. - virtual std::unique_ptr Extract( + virtual Expected> Extract( CarrierFormat format, const CarrierReader& reader) const = 0; // GlobalTracer returns the global tracer. diff --git a/c++11/include/opentracing/util.h b/c++11/include/opentracing/util.h index d3a00f0..2e94247 100644 --- a/c++11/include/opentracing/util.h +++ b/c++11/include/opentracing/util.h @@ -4,6 +4,7 @@ #include #include #include +#include namespace opentracing { inline namespace OPENTRACING_VERSION_NAMESPACE { @@ -12,7 +13,12 @@ using SteadyClock = std::chrono::steady_clock; using SystemTime = SystemClock::time_point; using SteadyTime = SteadyClock::time_point; -template +// Expected uses a C++11 implementation that follows the std::expected standard +// library proposal. +// +// See https://github.com/martinmoene/expected-lite +// https://github.com/viboes/std-make/blob/master/doc/proposal/expected/d0323r2.md +template using Expected = nonstd::expected; using nonstd::make_unexpected; diff --git a/c++11/src/noop.cpp b/c++11/src/noop.cpp index 62df584..a387a8a 100644 --- a/c++11/src/noop.cpp +++ b/c++11/src/noop.cpp @@ -41,15 +41,14 @@ class NoopTracer : public Tracer, NoopSpan(shared_from_this())); } - Expected Inject( - const SpanContext& sc, CarrierFormat format, - const CarrierWriter& writer) const override { + Expected Inject(const SpanContext& sc, CarrierFormat format, + const CarrierWriter& writer) const override { return {}; } - std::unique_ptr Extract( + Expected> Extract( CarrierFormat format, const CarrierReader& reader) const override { - return nullptr; + return std::unique_ptr(nullptr); } }; } // anonymous namespace diff --git a/c++11/src/propagation.cpp b/c++11/src/propagation.cpp new file mode 100644 index 0000000..68cf308 --- /dev/null +++ b/c++11/src/propagation.cpp @@ -0,0 +1,50 @@ +#include + +namespace opentracing { +inline namespace OPENTRACING_VERSION_NAMESPACE { +namespace { +class PropagationErrorCategory : public std::error_category { + public: + const char* name() const noexcept override { + return "OpenTracingPropagationError"; + } + + std::error_condition default_error_condition(int code) const + noexcept override { + if (code == unsupported_format_error.value()) + return std::make_error_condition(std::errc::not_supported); + else if (code == span_context_not_found_error.value()) + return std::make_error_condition(std::errc::invalid_argument); + else if (code == invalid_span_context_error.value()) + return std::make_error_condition(std::errc::not_supported); + else if (code == invalid_carrier_error.value()) + return std::make_error_condition(std::errc::invalid_argument); + else if (code == span_context_corrupted_error.value()) + return std::make_error_condition(std::errc::invalid_argument); + else + return std::error_condition(code, *this); + } + + std::string message(int code) const override { + if (code == unsupported_format_error.value()) + return "opentracing: Unknown or unsupported Inject/Extract format"; + else if (code == span_context_not_found_error.value()) + return "opentracing: SpanContext not found in Extract carrier"; + else if (code == invalid_span_context_error.value()) + return "opentracing: SpanContext type incompatible with tracer"; + else if (code == invalid_carrier_error.value()) + return "opentracing: Invalid Inject/Extract carrier"; + else if (code == span_context_corrupted_error.value()) + return "opentracing: SpanContext data corrupted in Extract carrier"; + else + return "opentracing: unknown propagation error"; + } +}; +} // anonymous namespace + +const std::error_category& propagation_error_category() { + static const PropagationErrorCategory error_category; + return error_category; +} +} // namespace OPENTRACING_VERSION_NAMESPACE +} // namespace opentracing From d103e34a6c80541d647443bf50bc6ff43abdf1f7 Mon Sep 17 00:00:00 2001 From: Ryan Burn Date: Tue, 13 Jun 2017 16:14:17 -0400 Subject: [PATCH 10/42] Added HTTPCarrier classes. --- c++11/include/opentracing/propagation.h | 68 +++++++++++++++---------- c++11/include/opentracing/tracer.h | 8 ++- c++11/src/propagation.cpp | 4 -- 3 files changed, 48 insertions(+), 32 deletions(-) diff --git a/c++11/include/opentracing/propagation.h b/c++11/include/opentracing/propagation.h index b8d3423..0132fa3 100644 --- a/c++11/include/opentracing/propagation.h +++ b/c++11/include/opentracing/propagation.h @@ -112,28 +112,22 @@ const std::error_category& propagation_error_category(); // implementation. const std::error_code unsupported_format_error(0, propagation_error_category()); -// `span_context_not_found_error` occurs when the `carrier` passed to -// Tracer::Extract() is valid and uncorrupted but has insufficient -// information to extract a SpanContext. -const std::error_code span_context_not_found_error( - 1, propagation_error_category()); - // `invalid_span_context_error` errors occur when Tracer::Inject() is asked to // operate on a SpanContext which it is not prepared to handle (for // example, since it was created by a different tracer implementation). -const std::error_code invalid_span_context_error(2, +const std::error_code invalid_span_context_error(1, propagation_error_category()); // `invalid_carrier_error` errors occur when Tracer::Inject() or // Tracer::Extract() // implementations expect a different type of `carrier` than they are // given. -const std::error_code invalid_carrier_error(3, propagation_error_category()); +const std::error_code invalid_carrier_error(2, propagation_error_category()); // `span_context_corrupted_error` occurs when the `carrier` passed to // Tracer::Extract() is of the expected type but is corrupted. const std::error_code span_context_corrupted_error( - 4, propagation_error_category()); + 3, propagation_error_category()); // Base class for implementation-dependent Tracer::Inject carrier-type // adapter. @@ -142,35 +136,55 @@ class CarrierReader { virtual ~CarrierReader() = default; }; -// Basic foundation for OpenTracing basictracer-compatible carrier readers. -class BasicCarrierReader : public CarrierReader { - public: - virtual void ForeachKey( - std::function f) const = 0; -}; - // Base class for implementation-dependent Tracer::Extract carrier-type adapter. class CarrierWriter { public: virtual ~CarrierWriter() = default; }; -// Basic foundation for OpenTracing basictracer-compatible carrier writers. -class BasicCarrierWriter : public CarrierWriter { - public: +// TextMapWriter is the Inject() carrier for the TextMap builtin format. With +// it, the caller can encode a SpanContext for propagation as entries in a map +// of unicode strings. +class TextMapReader : public CarrierReader { + // ForeachKey returns TextMap contents via repeated calls to the `f` + // function. If any call to `handler` returns an error, ForeachKey + // terminates and returns that error. + // + // NOTE: The backing store for the TextMapReader may contain data unrelated + // to SpanContext. As such, Inject() and Extract() implementations that + // call the TextMapWriter and TextMapReader interfaces must agree on a + // prefix or other convention to distinguish their own key:value pairs. + // + // The "foreach" callback pattern reduces unnecessary copying in some cases + // and also allows implementations to hold locks while the map is read. + virtual Expected ForeachKey( + std::function f) const = 0; +}; + +// TextMapWriter is the Inject() carrier for the TextMap builtin format. With +// it, the caller can encode a SpanContext for propagation as entries in a map +// of unicode strings. +class TextMapWriter : public CarrierWriter { + // Set a key:value pair to the carrier. Multiple calls to Set() for the + // same key leads to undefined behavior. + // + // NOTE: The backing store for the TextMapWriter may contain data unrelated + // to SpanContext. As such, Inject() and Extract() implementations that + // call the TextMapWriter and TextMapReader interfaces must agree on a + // prefix or other convention to distinguish their own key:value pairs. virtual Expected Set(const std::string& key, const std::string& value) const = 0; }; -// Base class for injecting into TextMap and HTTPHeaders carriers. -class TextMapReader : public BasicCarrierReader { - // TODO distinguish TextMap and HTTPHeaders behavior. -}; +// HTTPHeadersReader is the Inject() carrier for the HttpHeaders builtin format. +// With it, the caller can encode a SpanContext for propagation as entries in +// http request headers. +class HTTPHeadersReader : public TextMapReader {}; -// Base class for extracting from TextMap and HTTPHeaders carriers. -class TextMapWriter : public BasicCarrierWriter { - // TODO distinguish TextMap and HTTPHeaders behavior. -}; +// HTTPHeadersWriter is the Inject() carrier for the TextMap builtin format. +// With it, the caller can encode a SpanContext for propagation as entries in +// http request headers +class HTTPHeadersWriter : public TextMapWriter {}; } // namespace OPENTRACING_VERSION_NAMESPACE } // namespace opentracing diff --git a/c++11/include/opentracing/tracer.h b/c++11/include/opentracing/tracer.h index 6497c0e..d7a0264 100644 --- a/c++11/include/opentracing/tracer.h +++ b/c++11/include/opentracing/tracer.h @@ -92,6 +92,8 @@ class Tracer { // // OpenTracing defines a common set of `format` values (see BuiltinFormat), // and each has an expected carrier type. + // + // Throws only if `writer` does. virtual Expected Inject(const SpanContext& sc, CarrierFormat format, const CarrierWriter& writer) const = 0; @@ -100,7 +102,11 @@ class Tracer { // OpenTracing defines a common set of `format` values (see BuiltinFormat), // and each has an expected carrier type. // - // Returns a `SpanContext` that is `non-null` on success. + // Returns a `SpanContext` that is `non-null` on success or nullptr if + // no span is found; otherwise an std::error_code from + // propagation_error_category(). + // + // Throws only if `reader` does. virtual Expected> Extract( CarrierFormat format, const CarrierReader& reader) const = 0; diff --git a/c++11/src/propagation.cpp b/c++11/src/propagation.cpp index 68cf308..9f0d110 100644 --- a/c++11/src/propagation.cpp +++ b/c++11/src/propagation.cpp @@ -13,8 +13,6 @@ class PropagationErrorCategory : public std::error_category { noexcept override { if (code == unsupported_format_error.value()) return std::make_error_condition(std::errc::not_supported); - else if (code == span_context_not_found_error.value()) - return std::make_error_condition(std::errc::invalid_argument); else if (code == invalid_span_context_error.value()) return std::make_error_condition(std::errc::not_supported); else if (code == invalid_carrier_error.value()) @@ -28,8 +26,6 @@ class PropagationErrorCategory : public std::error_category { std::string message(int code) const override { if (code == unsupported_format_error.value()) return "opentracing: Unknown or unsupported Inject/Extract format"; - else if (code == span_context_not_found_error.value()) - return "opentracing: SpanContext not found in Extract carrier"; else if (code == invalid_span_context_error.value()) return "opentracing: SpanContext type incompatible with tracer"; else if (code == invalid_carrier_error.value()) From 133a0a86dd09f72188e9d5c0c40e4c586a224afe Mon Sep 17 00:00:00 2001 From: Ryan Burn Date: Tue, 13 Jun 2017 16:17:05 -0400 Subject: [PATCH 11/42] Use pointer instead of references for ChildOf/FollowsFrom functions. --- c++11/include/opentracing/propagation.h | 14 +++++++------- c++11/include/opentracing/tracer.h | 10 +++++----- c++11/test/noop_test.cpp | 2 +- 3 files changed, 13 insertions(+), 13 deletions(-) diff --git a/c++11/include/opentracing/propagation.h b/c++11/include/opentracing/propagation.h index 0132fa3..ac6ed87 100644 --- a/c++11/include/opentracing/propagation.h +++ b/c++11/include/opentracing/propagation.h @@ -165,13 +165,13 @@ class TextMapReader : public CarrierReader { // it, the caller can encode a SpanContext for propagation as entries in a map // of unicode strings. class TextMapWriter : public CarrierWriter { - // Set a key:value pair to the carrier. Multiple calls to Set() for the - // same key leads to undefined behavior. - // - // NOTE: The backing store for the TextMapWriter may contain data unrelated - // to SpanContext. As such, Inject() and Extract() implementations that - // call the TextMapWriter and TextMapReader interfaces must agree on a - // prefix or other convention to distinguish their own key:value pairs. + // Set a key:value pair to the carrier. Multiple calls to Set() for the + // same key leads to undefined behavior. + // + // NOTE: The backing store for the TextMapWriter may contain data unrelated + // to SpanContext. As such, Inject() and Extract() implementations that + // call the TextMapWriter and TextMapReader interfaces must agree on a + // prefix or other convention to distinguish their own key:value pairs. virtual Expected Set(const std::string& key, const std::string& value) const = 0; }; diff --git a/c++11/include/opentracing/tracer.h b/c++11/include/opentracing/tracer.h index d7a0264..22e11a2 100644 --- a/c++11/include/opentracing/tracer.h +++ b/c++11/include/opentracing/tracer.h @@ -154,27 +154,27 @@ class StartTimestamp : public StartSpanOption { // supported relationships. class SpanReference : public StartSpanOption { public: - SpanReference(SpanReferenceType type, const SpanContext& referenced) noexcept + SpanReference(SpanReferenceType type, const SpanContext* referenced) noexcept : type_(type), referenced_(referenced) {} SpanReference(const SpanReference& other) noexcept : StartSpanOption(), type_(other.type_), referenced_(other.referenced_) {} void Apply(StartSpanOptions& options) const noexcept override try { - options.references.emplace_back(type_, &referenced_); + if (referenced_) options.references.emplace_back(type_, referenced_); } catch (const std::bad_alloc&) { // Ignore reference if memory can't be allocated for it. } private: SpanReferenceType type_; - const SpanContext& referenced_; + const SpanContext* referenced_; }; // ChildOf returns a StartSpanOption pointing to a dependent parent span. // // See ChildOfRef, SpanReference -inline SpanReference ChildOf(const SpanContext& span_context) noexcept { +inline SpanReference ChildOf(const SpanContext* span_context) noexcept { return {SpanReferenceType::ChildOfRef, span_context}; } @@ -182,7 +182,7 @@ inline SpanReference ChildOf(const SpanContext& span_context) noexcept { // the child Span but does not directly depend on its result in any way. // // See FollowsFromRef, SpanReference -inline SpanReference FollowsFrom(const SpanContext& span_context) noexcept { +inline SpanReference FollowsFrom(const SpanContext* span_context) noexcept { return {SpanReferenceType::FollowsFromRef, span_context}; } diff --git a/c++11/test/noop_test.cpp b/c++11/test/noop_test.cpp index 56d1f2f..467c56f 100644 --- a/c++11/test/noop_test.cpp +++ b/c++11/test/noop_test.cpp @@ -9,7 +9,7 @@ int main() { auto span1 = tracer->StartSpan("a"); assert(&span1->tracer() == tracer.get()); - auto span2 = tracer->StartSpan("b", {ChildOf(span1->context())}); + auto span2 = tracer->StartSpan("b", {ChildOf(&span1->context())}); span2->SetOperationName("b1"); span2->SetTag("x", true); assert(span2->BaggageItem("y").empty()); From 2e19d8ed6d9a65b1eb88cb3332fb12bbdafcaef2 Mon Sep 17 00:00:00 2001 From: Ryan Burn Date: Wed, 14 Jun 2017 11:11:44 -0400 Subject: [PATCH 12/42] Added header file for versioning. --- CMakeLists.txt | 2 ++ c++11/CMakeLists.txt | 5 +++++ c++11/include/opentracing/noop.h | 6 +++--- c++11/include/opentracing/preprocessor.h | 7 ------- c++11/include/opentracing/propagation.h | 11 +++++------ c++11/include/opentracing/span.h | 6 +++--- c++11/include/opentracing/stringref.h | 6 +++--- c++11/include/opentracing/tracer.h | 6 +++--- c++11/include/opentracing/util.h | 6 +++--- c++11/include/opentracing/value.h | 6 +++--- c++11/src/noop.cpp | 4 ++-- c++11/src/propagation.cpp | 4 ++-- c++11/src/tracer.cpp | 4 ++-- c++11/version.h.in | 10 ++++++++++ 14 files changed, 46 insertions(+), 37 deletions(-) delete mode 100644 c++11/include/opentracing/preprocessor.h create mode 100644 c++11/version.h.in diff --git a/CMakeLists.txt b/CMakeLists.txt index a3e330c..1a81d20 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -3,6 +3,8 @@ enable_testing() project(opentracing-cpp) +set(OPENTRACING_ABI_VERSION "1") + SET(CPACK_PACKAGE_DESCRIPTION_SUMMARY "C++ implementation of the OpenTracing API") SET(CPACK_PACKAGE_VENDOR "opentracing.io") SET(CPACK_PACKAGE_DESCRIPTION_FILE "${CMAKE_CURRENT_SOURCE_DIR}/README.md") diff --git a/c++11/CMakeLists.txt b/c++11/CMakeLists.txt index d8bf9cd..191031c 100644 --- a/c++11/CMakeLists.txt +++ b/c++11/CMakeLists.txt @@ -12,6 +12,11 @@ elseif ("${CMAKE_CXX_COMPILER_ID}" STREQUAL "GNU") set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -Wall -Wextra -Wno-unused-parameter") endif() +configure_file(version.h.in include/opentracing/version.h) +include_directories(${CMAKE_CURRENT_BINARY_DIR}/include) +install(DIRECTORY ${CMAKE_CURRENT_BINARY_DIR}/include/opentracing + DESTINATION include) + include_directories(SYSTEM 3rd_party/include) install(DIRECTORY 3rd_party/include/opentracing DESTINATION include FILES_MATCHING PATTERN "*.hpp" diff --git a/c++11/include/opentracing/noop.h b/c++11/include/opentracing/noop.h index 099c93e..58b806b 100644 --- a/c++11/include/opentracing/noop.h +++ b/c++11/include/opentracing/noop.h @@ -1,12 +1,12 @@ #ifndef OPENTRACING_NOOP_H #define OPENTRACING_NOOP_H -#include #include +#include #include namespace opentracing { -inline namespace OPENTRACING_VERSION_NAMESPACE { +inline namespace OPENTRACING_INLINE_NAMESPACE { // A NoopTracer is a trivial, minimum overhead implementation of Tracer // for which all operations are no-ops. // @@ -21,7 +21,7 @@ inline namespace OPENTRACING_VERSION_NAMESPACE { // // WARNING: NoopTracer does not support baggage propagation. std::shared_ptr make_noop_tracer() noexcept; -} // namespace OPENTRACING_VERSION_NAMESPACE +} // namespace OPENTRACING_INLINE_NAMESPACE } // namespace opentracing #endif // OPENTRACING_NOOP_H diff --git a/c++11/include/opentracing/preprocessor.h b/c++11/include/opentracing/preprocessor.h deleted file mode 100644 index be4cfdc..0000000 --- a/c++11/include/opentracing/preprocessor.h +++ /dev/null @@ -1,7 +0,0 @@ -#ifndef OPENTRACING_PREPROCESSOR_H -#define OPENTRACING_PREPROCESSOR_H - -// Increase when an ABI-breaking change is made. -#define OPENTRACING_VERSION_NAMESPACE v1 - -#endif // OPENTRACING_PREPROCESSOR_H diff --git a/c++11/include/opentracing/propagation.h b/c++11/include/opentracing/propagation.h index ac6ed87..5d30d71 100644 --- a/c++11/include/opentracing/propagation.h +++ b/c++11/include/opentracing/propagation.h @@ -1,15 +1,15 @@ #ifndef OPENTRACING_PROPAGATION_H #define OPENTRACING_PROPAGATION_H -#include #include #include +#include #include #include #include namespace opentracing { -inline namespace OPENTRACING_VERSION_NAMESPACE { +inline namespace OPENTRACING_INLINE_NAMESPACE { enum class SpanReferenceType { // ChildOfRef refers to a parent Span that caused *and* somehow depends // upon the new child Span. Often (but not always), the parent Span cannot @@ -119,9 +119,8 @@ const std::error_code invalid_span_context_error(1, propagation_error_category()); // `invalid_carrier_error` errors occur when Tracer::Inject() or -// Tracer::Extract() -// implementations expect a different type of `carrier` than they are -// given. +// Tracer::Extract() implementations expect a different type of `carrier` than +// they are given. const std::error_code invalid_carrier_error(2, propagation_error_category()); // `span_context_corrupted_error` occurs when the `carrier` passed to @@ -185,7 +184,7 @@ class HTTPHeadersReader : public TextMapReader {}; // With it, the caller can encode a SpanContext for propagation as entries in // http request headers class HTTPHeadersWriter : public TextMapWriter {}; -} // namespace OPENTRACING_VERSION_NAMESPACE +} // namespace OPENTRACING_INLINE_NAMESPACE } // namespace opentracing #endif // OPENTRACING_PROPAGATION_H diff --git a/c++11/include/opentracing/span.h b/c++11/include/opentracing/span.h index b225a27..017ff8d 100644 --- a/c++11/include/opentracing/span.h +++ b/c++11/include/opentracing/span.h @@ -1,16 +1,16 @@ #ifndef OPENTRACING_SPAN_H #define OPENTRACING_SPAN_H -#include #include #include #include +#include #include #include #include namespace opentracing { -inline namespace OPENTRACING_VERSION_NAMESPACE { +inline namespace OPENTRACING_INLINE_NAMESPACE { class Tracer; // SpanContext represents Span state that must propagate to descendant Spans and @@ -141,7 +141,7 @@ class FinishTimestamp : public FinishSpanOption { private: SteadyTime steady_when_; }; -} // namespace OPENTRACING_VERSION_NAMESPACE +} // namespace OPENTRACING_INLINE_NAMESPACE } // namespace opentracing #endif // OPENTRACING_SPAN_H diff --git a/c++11/include/opentracing/stringref.h b/c++11/include/opentracing/stringref.h index 8214123..a173268 100644 --- a/c++11/include/opentracing/stringref.h +++ b/c++11/include/opentracing/stringref.h @@ -1,7 +1,7 @@ #ifndef OPENTRACING_STRINGREF_H #define OPENTRACING_STRINGREF_H -#include +#include #include #include #include @@ -36,7 +36,7 @@ // sudden. That design issue still needs to be addressed. namespace opentracing { -inline namespace OPENTRACING_VERSION_NAMESPACE { +inline namespace OPENTRACING_INLINE_NAMESPACE { // =============== // class StringRef // =============== @@ -120,7 +120,7 @@ inline std::ostream& operator<<(std::ostream& os, return os.write(ref.data(), static_cast(ref.length())); } -} // namespace OPENTRACING_VERSION_NAMESPACE +} // namespace OPENTRACING_INLINE_NAMESPACE } // namespace opentracing #endif // OPENTRACING_STRINGREF_H diff --git a/c++11/include/opentracing/tracer.h b/c++11/include/opentracing/tracer.h index 22e11a2..ed2f05b 100644 --- a/c++11/include/opentracing/tracer.h +++ b/c++11/include/opentracing/tracer.h @@ -1,11 +1,11 @@ #ifndef OPENTRACING_TRACER_H #define OPENTRACING_TRACER_H -#include #include #include #include #include +#include #include #include #include @@ -13,7 +13,7 @@ #include namespace opentracing { -inline namespace OPENTRACING_VERSION_NAMESPACE { +inline namespace OPENTRACING_INLINE_NAMESPACE { // StartSpanOptions allows Tracer.StartSpan() callers a mechanism to override // the start timestamp, specify Span References, and make a single Tag or // multiple Tags available at Span start time. @@ -208,7 +208,7 @@ class SetTag : public StartSpanOption { StringRef key_; const Value& value_; }; -} // namespace OPENTRACING_VERSION_NAMESPACE +} // namespace OPENTRACING_INLINE_NAMESPACE } // namespace opentracing #endif // OPENTRACING_TRACER_H diff --git a/c++11/include/opentracing/util.h b/c++11/include/opentracing/util.h index 2e94247..375f927 100644 --- a/c++11/include/opentracing/util.h +++ b/c++11/include/opentracing/util.h @@ -1,13 +1,13 @@ #ifndef OPENTRACING_UTIL_H #define OPENTRACING_UTIL_H -#include +#include #include #include #include namespace opentracing { -inline namespace OPENTRACING_VERSION_NAMESPACE { +inline namespace OPENTRACING_INLINE_NAMESPACE { using SystemClock = std::chrono::system_clock; using SteadyClock = std::chrono::steady_clock; using SystemTime = SystemClock::time_point; @@ -38,7 +38,7 @@ class option_wrapper { private: const T *ptr_; }; -} // namespace OPENTRACING_VERSION_NAMESPACE +} // namespace OPENTRACING_INLINE_NAMESPACE } // namespace opentracing #endif // OPENTRACING_UTIL_H diff --git a/c++11/include/opentracing/value.h b/c++11/include/opentracing/value.h index afbf19b..1df12d7 100644 --- a/c++11/include/opentracing/value.h +++ b/c++11/include/opentracing/value.h @@ -1,8 +1,8 @@ #ifndef OPENTRACING_VALUE_H #define OPENTRACING_VALUE_H -#include #include +#include #include #include #include @@ -10,7 +10,7 @@ #include namespace opentracing { -inline namespace OPENTRACING_VERSION_NAMESPACE { +inline namespace OPENTRACING_INLINE_NAMESPACE { // Variant value types for span tags and log payloads. class Value; @@ -63,7 +63,7 @@ class Value : public variant_type { Value(const Dictionary& values) : variant_type(values) {} Value(Dictionary&& values) : variant_type(std::move(values)) {} }; -} // namespace OPENTRACING_VERSION_NAMESPACE +} // namespace OPENTRACING_INLINE_NAMESPACE } // namespace opentracing #endif // OPENTRACING_VALUE_H diff --git a/c++11/src/noop.cpp b/c++11/src/noop.cpp index a387a8a..68bd239 100644 --- a/c++11/src/noop.cpp +++ b/c++11/src/noop.cpp @@ -1,7 +1,7 @@ #include namespace opentracing { -inline namespace OPENTRACING_VERSION_NAMESPACE { +inline namespace OPENTRACING_INLINE_NAMESPACE { namespace { class NoopSpanContext : public SpanContext { public: @@ -56,5 +56,5 @@ class NoopTracer : public Tracer, std::shared_ptr make_noop_tracer() noexcept { return std::shared_ptr(new (std::nothrow) NoopTracer()); } -} // namespace OPENTRACING_VERSION_NAMESPACE +} // namespace OPENTRACING_INLINE_NAMESPACE } // namesapce opentracing diff --git a/c++11/src/propagation.cpp b/c++11/src/propagation.cpp index 9f0d110..fb71253 100644 --- a/c++11/src/propagation.cpp +++ b/c++11/src/propagation.cpp @@ -1,7 +1,7 @@ #include namespace opentracing { -inline namespace OPENTRACING_VERSION_NAMESPACE { +inline namespace OPENTRACING_INLINE_NAMESPACE { namespace { class PropagationErrorCategory : public std::error_category { public: @@ -42,5 +42,5 @@ const std::error_category& propagation_error_category() { static const PropagationErrorCategory error_category; return error_category; } -} // namespace OPENTRACING_VERSION_NAMESPACE +} // namespace OPENTRACING_INLINE_NAMESPACE } // namespace opentracing diff --git a/c++11/src/tracer.cpp b/c++11/src/tracer.cpp index da16ac2..17c47ac 100644 --- a/c++11/src/tracer.cpp +++ b/c++11/src/tracer.cpp @@ -2,7 +2,7 @@ #include namespace opentracing { -inline namespace OPENTRACING_VERSION_NAMESPACE { +inline namespace OPENTRACING_INLINE_NAMESPACE { static std::shared_ptr& get_global_tracer() { static std::shared_ptr global_tracer = make_noop_tracer(); return global_tracer; @@ -17,5 +17,5 @@ std::shared_ptr Tracer::InitGlobal( get_global_tracer().swap(tracer); return tracer; } -} // namespace OPENTRACING_VERSION_NAMESPACE +} // namespace OPENTRACING_INLINE_NAMESPACE } // namespace opentracing diff --git a/c++11/version.h.in b/c++11/version.h.in new file mode 100644 index 0000000..a407253 --- /dev/null +++ b/c++11/version.h.in @@ -0,0 +1,10 @@ +#ifndef OPENTRACING_VERSION_H +#define OPENTRACING_VERSION_H + +#define OPENTRACING_INLINE_NAMESPACE v${OPENTRACING_ABI_VERSION} + +#define OPENTRACING_VERSION "${CPACK_PACKAGE_VERSION_MAJOR}." \ + "${CPACK_PACKAGE_VERSION_MINOR}." \ + "${CPACK_PACKAGE_VERSION_PATCH}" + +#endif // OPENTRACING_VERSION_H From 270eb30e53a68a50a43ee4a2e121407223b9e9e3 Mon Sep 17 00:00:00 2001 From: Ryan Burn Date: Wed, 14 Jun 2017 11:52:39 -0400 Subject: [PATCH 13/42] Allow the Foreach functor to return an error. --- c++11/include/opentracing/propagation.h | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/c++11/include/opentracing/propagation.h b/c++11/include/opentracing/propagation.h index 5d30d71..cb8f1e1 100644 --- a/c++11/include/opentracing/propagation.h +++ b/c++11/include/opentracing/propagation.h @@ -146,8 +146,8 @@ class CarrierWriter { // of unicode strings. class TextMapReader : public CarrierReader { // ForeachKey returns TextMap contents via repeated calls to the `f` - // function. If any call to `handler` returns an error, ForeachKey - // terminates and returns that error. + // function. If any call to `f` returns an error, ForeachKey terminates and + // returns that error. // // NOTE: The backing store for the TextMapReader may contain data unrelated // to SpanContext. As such, Inject() and Extract() implementations that @@ -157,7 +157,8 @@ class TextMapReader : public CarrierReader { // The "foreach" callback pattern reduces unnecessary copying in some cases // and also allows implementations to hold locks while the map is read. virtual Expected ForeachKey( - std::function f) const = 0; + std::function(StringRef key, StringRef value)> f) + const = 0; }; // TextMapWriter is the Inject() carrier for the TextMap builtin format. With From b13d7f4bbdefe711175c2f169b87e0f3d44d995a Mon Sep 17 00:00:00 2001 From: Ryan Burn Date: Wed, 14 Jun 2017 23:27:29 -0400 Subject: [PATCH 14/42] Correct time_point bug. --- c++11/include/opentracing/tracer.h | 15 ++++++++++----- c++11/include/opentracing/util.h | 13 +++++++++++++ 2 files changed, 23 insertions(+), 5 deletions(-) diff --git a/c++11/include/opentracing/tracer.h b/c++11/include/opentracing/tracer.h index ed2f05b..86afd3a 100644 --- a/c++11/include/opentracing/tracer.h +++ b/c++11/include/opentracing/tracer.h @@ -126,13 +126,18 @@ class StartTimestamp : public StartSpanOption { StartTimestamp(SystemTime system_when, SteadyTime steady_when) noexcept : system_when_(system_when), steady_when_(steady_when) {} + // Construct a timestamp using a duration from the epoch of std::time_t. + // From the documentation on std::time_t's epoch: + // Although not defined, this is almost always an integral value holding + // the number of seconds (not counting leap seconds) since 00:00, Jan 1 + // 1970 UTC, corresponding to POSIX time + // See http://en.cppreference.com/w/cpp/chrono/c/time_t template explicit StartTimestamp( - const std::chrono::duration& time_since_epoch) noexcept - : system_when_(std::chrono::duration_cast( - time_since_epoch)), - steady_when_(std::chrono::duration_cast( - time_since_epoch)) {} + const std::chrono::duration& time_since_epoch) noexcept { + system_when_ = SystemClock::from_time_t(std::time_t(0)) + time_since_epoch; + steady_when_ = convert_time_point(system_when_); + } StartTimestamp(const StartTimestamp& other) noexcept : StartSpanOption(), diff --git a/c++11/include/opentracing/util.h b/c++11/include/opentracing/util.h index 375f927..bffdee2 100644 --- a/c++11/include/opentracing/util.h +++ b/c++11/include/opentracing/util.h @@ -38,6 +38,19 @@ class option_wrapper { private: const T *ptr_; }; + +// Support conversion between time_points from different clocks. There's no +// standard way to get the difference in epochs between clocks, so this uses +// an approximation suggested by Howard Hinnant. +// +// See https://stackoverflow.com/a/35282833/4447365 +template +typename ToClock::time_point convert_time_point( + std::chrono::time_point src_time_point) { + auto from_now = FromClock::now(); + auto to_now = ToClock::now(); + return to_now + (src_time_point - from_now); +} } // namespace OPENTRACING_INLINE_NAMESPACE } // namespace opentracing From 3f8f218fb778ee2052020681c4caae2e77a2bd57 Mon Sep 17 00:00:00 2001 From: Ryan Burn Date: Wed, 14 Jun 2017 23:38:47 -0400 Subject: [PATCH 15/42] Support different native durations in convert_time_point. --- c++11/include/opentracing/util.h | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/c++11/include/opentracing/util.h b/c++11/include/opentracing/util.h index bffdee2..4073078 100644 --- a/c++11/include/opentracing/util.h +++ b/c++11/include/opentracing/util.h @@ -46,10 +46,11 @@ class option_wrapper { // See https://stackoverflow.com/a/35282833/4447365 template typename ToClock::time_point convert_time_point( - std::chrono::time_point src_time_point) { + std::chrono::time_point from_time_point) { auto from_now = FromClock::now(); auto to_now = ToClock::now(); - return to_now + (src_time_point - from_now); + return to_now + std::chrono::duration_cast( + from_time_point - from_now); } } // namespace OPENTRACING_INLINE_NAMESPACE } // namespace opentracing From 0fe9cee1d560f09ed304e0f7bd60796b02c3032b Mon Sep 17 00:00:00 2001 From: Ryan Burn Date: Wed, 14 Jun 2017 23:45:21 -0400 Subject: [PATCH 16/42] Correction for time specification using epoch. --- c++11/include/opentracing/tracer.h | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/c++11/include/opentracing/tracer.h b/c++11/include/opentracing/tracer.h index 86afd3a..3f13be1 100644 --- a/c++11/include/opentracing/tracer.h +++ b/c++11/include/opentracing/tracer.h @@ -135,7 +135,9 @@ class StartTimestamp : public StartSpanOption { template explicit StartTimestamp( const std::chrono::duration& time_since_epoch) noexcept { - system_when_ = SystemClock::from_time_t(std::time_t(0)) + time_since_epoch; + system_when_ = + SystemClock::from_time_t(std::time_t(0)) + + std::chrono::duration_cast(time_since_epoch); steady_when_ = convert_time_point(system_when_); } From f961c79695ac5fdf53f54f6375b968fb62ed5bcb Mon Sep 17 00:00:00 2001 From: Ryan Burn Date: Thu, 15 Jun 2017 13:22:08 -0400 Subject: [PATCH 17/42] Correct permissions in Carrier classes. --- c++11/include/opentracing/propagation.h | 2 ++ 1 file changed, 2 insertions(+) diff --git a/c++11/include/opentracing/propagation.h b/c++11/include/opentracing/propagation.h index cb8f1e1..3e97826 100644 --- a/c++11/include/opentracing/propagation.h +++ b/c++11/include/opentracing/propagation.h @@ -145,6 +145,7 @@ class CarrierWriter { // it, the caller can encode a SpanContext for propagation as entries in a map // of unicode strings. class TextMapReader : public CarrierReader { + public: // ForeachKey returns TextMap contents via repeated calls to the `f` // function. If any call to `f` returns an error, ForeachKey terminates and // returns that error. @@ -165,6 +166,7 @@ class TextMapReader : public CarrierReader { // it, the caller can encode a SpanContext for propagation as entries in a map // of unicode strings. class TextMapWriter : public CarrierWriter { + public: // Set a key:value pair to the carrier. Multiple calls to Set() for the // same key leads to undefined behavior. // From 14d9471ed6ed61eb89dd015b1e278bbc0acc3804 Mon Sep 17 00:00:00 2001 From: Ryan Burn Date: Thu, 15 Jun 2017 18:29:12 -0400 Subject: [PATCH 18/42] Temporary work-around to https://github.com/martinmoene/expected-lite/issues/8. --- .../include/opentracing/martinmoene_expected/expected.hpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/c++11/3rd_party/include/opentracing/martinmoene_expected/expected.hpp b/c++11/3rd_party/include/opentracing/martinmoene_expected/expected.hpp index 99fd10f..ce99429 100644 --- a/c++11/3rd_party/include/opentracing/martinmoene_expected/expected.hpp +++ b/c++11/3rd_party/include/opentracing/martinmoene_expected/expected.hpp @@ -866,7 +866,7 @@ class expected void swap( expected & rhs ) noexcept ( - std::is_nothrow_move_constructible::value && noexcept( swap( std::declval(), std::declval() ) ) + std::is_nothrow_move_constructible::value && noexcept( std::swap( std::declval(), std::declval() ) ) ) { using std::swap; From 9942fcac5a57b1fb57bdac9293a682344393ccce Mon Sep 17 00:00:00 2001 From: Ryan Burn Date: Thu, 15 Jun 2017 19:29:24 -0400 Subject: [PATCH 19/42] Added equality operators to StringRef. --- c++11/include/opentracing/stringref.h | 42 +++++++++++++++++++++++++++ c++11/test/stringref_test.cpp | 4 +-- 2 files changed, 44 insertions(+), 2 deletions(-) diff --git a/c++11/include/opentracing/stringref.h b/c++11/include/opentracing/stringref.h index a173268..f681470 100644 --- a/c++11/include/opentracing/stringref.h +++ b/c++11/include/opentracing/stringref.h @@ -2,6 +2,7 @@ #define OPENTRACING_STRINGREF_H #include +#include #include #include #include @@ -115,6 +116,47 @@ class StringRef { size_t length_; // Length of data pointed to by 'data_' }; +inline bool operator==(StringRef lhs, StringRef rhs) noexcept { + return lhs.length() == rhs.length() && + std::equal(lhs.data(), lhs.data() + lhs.length(), rhs.data()); +} + +inline bool operator==(StringRef lhs, const std::string& rhs) noexcept { + return lhs == StringRef(rhs); +} + +inline bool operator==(const std::string& lhs, StringRef rhs) noexcept { + return StringRef(lhs) == rhs; +} + +inline bool operator==(StringRef lhs, const char* rhs) noexcept { + return lhs == StringRef(rhs); +} + +inline bool operator==(const char* lhs, StringRef rhs) noexcept { + return StringRef(lhs) == rhs; +} + +inline bool operator!=(StringRef lhs, StringRef rhs) noexcept { + return !(lhs == rhs); +} + +inline bool operator!=(StringRef lhs, const std::string& rhs) noexcept { + return !(lhs == rhs); +} + +inline bool operator!=(const std::string& lhs, StringRef rhs) noexcept { + return !(lhs == rhs); +} + +inline bool operator!=(StringRef lhs, const char* rhs) noexcept { + return !(lhs == rhs); +} + +inline bool operator!=(const char* lhs, StringRef rhs) noexcept { + return !(lhs == rhs); +} + inline std::ostream& operator<<(std::ostream& os, const opentracing::StringRef& ref) { return os.write(ref.data(), static_cast(ref.length())); diff --git a/c++11/test/stringref_test.cpp b/c++11/test/stringref_test.cpp index 8bd92e5..7927fb2 100644 --- a/c++11/test/stringref_test.cpp +++ b/c++11/test/stringref_test.cpp @@ -54,7 +54,7 @@ static void test_reset() { ref.reset("hello world"); - assert(std::string("hello world") == std::string(ref)); + assert("hello world" == ref); assert(std::string("hello world") == std::string(ref.data())); assert(std::strlen("hello world") == ref.length()); @@ -85,7 +85,7 @@ static void test_reset() { ref.reset(p, std::strlen(p)); assert(p == ref.data()); assert(std::strlen(p) == ref.length()); - assert(std::string(p) == std::string(ref)); + assert(p == ref); assert(std::string(p) == std::string(ref.data())); } From 0c9500de84c493197e607b35b703c2dcce5304a3 Mon Sep 17 00:00:00 2001 From: Ryan Burn Date: Thu, 15 Jun 2017 22:15:37 -0400 Subject: [PATCH 20/42] Added begin/end functions to StringRef. --- c++11/include/opentracing/stringref.h | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/c++11/include/opentracing/stringref.h b/c++11/include/opentracing/stringref.h index f681470..541262a 100644 --- a/c++11/include/opentracing/stringref.h +++ b/c++11/include/opentracing/stringref.h @@ -111,6 +111,12 @@ class StringRef { // Return the length of the referenced string size_t length() const noexcept { return length_; } + // Returns a RandomAccessIterator to the first element. + const char* begin() const noexcept { return data(); } + + // Returns a RandomAccessIterator for the last element. + const char* end() const noexcept { return data() + length(); } + private: const char* data_; // Pointer to external storage size_t length_; // Length of data pointed to by 'data_' From 5bca4fe1f8119311be5362e573cea5f2179c3805 Mon Sep 17 00:00:00 2001 From: Ryan Burn Date: Thu, 15 Jun 2017 22:19:15 -0400 Subject: [PATCH 21/42] Support specification of StartTime with SystemClock::time_point. --- c++11/include/opentracing/tracer.h | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/c++11/include/opentracing/tracer.h b/c++11/include/opentracing/tracer.h index 3f13be1..24e2ef8 100644 --- a/c++11/include/opentracing/tracer.h +++ b/c++11/include/opentracing/tracer.h @@ -126,6 +126,10 @@ class StartTimestamp : public StartSpanOption { StartTimestamp(SystemTime system_when, SteadyTime steady_when) noexcept : system_when_(system_when), steady_when_(steady_when) {} + StartTimestamp(SystemTime system_when) noexcept + : system_when_(system_when), + steady_when_(convert_time_point(system_when_)) {} + // Construct a timestamp using a duration from the epoch of std::time_t. // From the documentation on std::time_t's epoch: // Although not defined, this is almost always an integral value holding @@ -134,12 +138,10 @@ class StartTimestamp : public StartSpanOption { // See http://en.cppreference.com/w/cpp/chrono/c/time_t template explicit StartTimestamp( - const std::chrono::duration& time_since_epoch) noexcept { - system_when_ = - SystemClock::from_time_t(std::time_t(0)) + - std::chrono::duration_cast(time_since_epoch); - steady_when_ = convert_time_point(system_when_); - } + const std::chrono::duration& time_since_epoch) noexcept + : StartTimestamp(SystemClock::from_time_t(std::time_t(0)) + + std::chrono::duration_cast( + time_since_epoch)) {} StartTimestamp(const StartTimestamp& other) noexcept : StartSpanOption(), From 6469ffd9c64a7b483e0844549184ef82bbdfe416 Mon Sep 17 00:00:00 2001 From: Ryan Burn Date: Fri, 16 Jun 2017 13:19:36 -0400 Subject: [PATCH 22/42] Added a logging interface. --- c++11/include/opentracing/span.h | 10 ++++++++++ c++11/src/noop.cpp | 2 ++ c++11/test/noop_test.cpp | 1 + 3 files changed, 13 insertions(+) diff --git a/c++11/include/opentracing/span.h b/c++11/include/opentracing/span.h index 017ff8d..7a4df48 100644 --- a/c++11/include/opentracing/span.h +++ b/c++11/include/opentracing/span.h @@ -110,6 +110,16 @@ class Span { // if the value isn't found in this Span. virtual std::string BaggageItem(StringRef restricted_key) const noexcept = 0; + // Log is an efficient and type-checked way to record key:value logging data + // about a Span. Here's an example: + // + // span.Log({ + // {"event", "soft error"}, + // {"type", "cache timeout"}, + // {"waited.millis", 1500}}); + virtual void Log( + std::initializer_list> fields) noexcept = 0; + // context() yields the SpanContext for this Span. Note that the return // value of context() is still valid after a call to Span.Finish(), as is // a call to Span.context() after a call to Span.Finish(). diff --git a/c++11/src/noop.cpp b/c++11/src/noop.cpp index 68bd239..78ed4fa 100644 --- a/c++11/src/noop.cpp +++ b/c++11/src/noop.cpp @@ -23,6 +23,8 @@ class NoopSpan : public Span { std::string BaggageItem(StringRef restricted_key) const noexcept override { return {}; } + void Log(std::initializer_list> + fields) noexcept override {} const SpanContext& context() const noexcept override { return span_context_; } const Tracer& tracer() const noexcept override { return *tracer_; } diff --git a/c++11/test/noop_test.cpp b/c++11/test/noop_test.cpp index 467c56f..67dfa91 100644 --- a/c++11/test/noop_test.cpp +++ b/c++11/test/noop_test.cpp @@ -13,6 +13,7 @@ int main() { span2->SetOperationName("b1"); span2->SetTag("x", true); assert(span2->BaggageItem("y").empty()); + span2->Log({{"event", "xyz"}, {"abc", 123}}); span2->Finish(); return 0; From bb6be9271479e429c77f20bd78b75487aa4eed74 Mon Sep 17 00:00:00 2001 From: Ryan Burn Date: Fri, 16 Jun 2017 13:20:31 -0400 Subject: [PATCH 23/42] Exclude 3rd_party code from clang-format. --- scripts/run_clang_format.sh | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/scripts/run_clang_format.sh b/scripts/run_clang_format.sh index 24dbf04..94c52ed 100755 --- a/scripts/run_clang_format.sh +++ b/scripts/run_clang_format.sh @@ -1,2 +1,3 @@ #!/bin/sh -find . \( -name '*.h' -or -name '*.cpp' \) -exec clang-format -i {} \; +find . -path ./c++11/3rd_party -prune -o \( -name '*.h' -or -name '*.cpp' \) \ + -exec clang-format -i {} \; From 10793fb0495abed99130983124bcea44e6db2f69 Mon Sep 17 00:00:00 2001 From: Ryan Burn Date: Fri, 16 Jun 2017 14:23:48 -0400 Subject: [PATCH 24/42] Add a Close method to Tracer. --- c++11/include/opentracing/tracer.h | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/c++11/include/opentracing/tracer.h b/c++11/include/opentracing/tracer.h index 24e2ef8..201f194 100644 --- a/c++11/include/opentracing/tracer.h +++ b/c++11/include/opentracing/tracer.h @@ -71,6 +71,8 @@ class Tracer { // opentracing::Tag{"user_agent", loggedReq.UserAgent}, // opentracing::StartTimestamp(loggedReq.timestamp())}) // + // If StartSpan is called after Close it leaves the Tracer in a valid + // state, but its behavior is unspecified. std::unique_ptr StartSpan( StringRef operation_name, std::initializer_list> option_list = {}) @@ -110,6 +112,13 @@ class Tracer { virtual Expected> Extract( CarrierFormat format, const CarrierReader& reader) const = 0; + // Close is called when a tracer is finished processing spans. It is not + // required to be called and its effect is unspecified. For example, an + // implementation might use this function to flush buffered spans to its + // recording system and failing to call it could result in some spans being + // dropped. + virtual void Close() noexcept {} + // GlobalTracer returns the global tracer. static std::shared_ptr Global() noexcept; From 843ba18cb029faa6a70d981fa153d5043ebd3290 Mon Sep 17 00:00:00 2001 From: Ryan Burn Date: Fri, 16 Jun 2017 17:02:33 -0400 Subject: [PATCH 25/42] Require a Span's destructor to call finish if not done so already. --- c++11/include/opentracing/span.h | 2 ++ 1 file changed, 2 insertions(+) diff --git a/c++11/include/opentracing/span.h b/c++11/include/opentracing/span.h index 7a4df48..7dc12b7 100644 --- a/c++11/include/opentracing/span.h +++ b/c++11/include/opentracing/span.h @@ -51,6 +51,8 @@ class FinishSpanOption { // Spans are created by the Tracer interface. class Span { public: + // If Finish has not already been called for the Span, it's destructor must + // do so. virtual ~Span() = default; // Sets the end timestamp and finalizes Span state. From 3de9ab8f4379426503930205cde7280f6e91ac56 Mon Sep 17 00:00:00 2001 From: Ryan Burn Date: Fri, 16 Jun 2017 17:26:39 -0400 Subject: [PATCH 26/42] Fix specification of FinishTimestamp. --- c++11/include/opentracing/span.h | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/c++11/include/opentracing/span.h b/c++11/include/opentracing/span.h index 7dc12b7..0ca6699 100644 --- a/c++11/include/opentracing/span.h +++ b/c++11/include/opentracing/span.h @@ -138,10 +138,18 @@ class FinishTimestamp : public FinishSpanOption { explicit FinishTimestamp(SteadyTime steady_when) noexcept : steady_when_(steady_when) {} + // Construct a timestamp using a duration from the epoch of std::time_t. + // From the documentation on std::time_t's epoch: + // Although not defined, this is almost always an integral value holding + // the number of seconds (not counting leap seconds) since 00:00, Jan 1 + // 1970 UTC, corresponding to POSIX time + // See http://en.cppreference.com/w/cpp/chrono/c/time_t template explicit FinishTimestamp( const std::chrono::duration& time_since_epoch) noexcept - : steady_when_(time_since_epoch) {} + : steady_when_(SystemClock::from_time_t(std::time_t(0)) + + std::chrono::duration_cast( + time_since_epoch)) {} FinishTimestamp(const FinishTimestamp& other) noexcept : FinishSpanOption(), steady_when_(other.steady_when_) {} From 31c209ecf74ef704e1423637d4359474040b87eb Mon Sep 17 00:00:00 2001 From: Ryan Burn Date: Fri, 16 Jun 2017 17:41:00 -0400 Subject: [PATCH 27/42] Convert timestamp to steady clock in FinishTimestamp. --- c++11/include/opentracing/span.h | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/c++11/include/opentracing/span.h b/c++11/include/opentracing/span.h index 0ca6699..f18bc57 100644 --- a/c++11/include/opentracing/span.h +++ b/c++11/include/opentracing/span.h @@ -147,9 +147,10 @@ class FinishTimestamp : public FinishSpanOption { template explicit FinishTimestamp( const std::chrono::duration& time_since_epoch) noexcept - : steady_when_(SystemClock::from_time_t(std::time_t(0)) + - std::chrono::duration_cast( - time_since_epoch)) {} + : steady_when_(convert_time_point( + SystemClock::from_time_t(std::time_t(0)) + + std::chrono::duration_cast( + time_since_epoch))) {} FinishTimestamp(const FinishTimestamp& other) noexcept : FinishSpanOption(), steady_when_(other.steady_when_) {} From 8d02ac52b40e7ce7dc54f0a94954bec89a58524a Mon Sep 17 00:00:00 2001 From: Ryan Burn Date: Fri, 16 Jun 2017 17:54:53 -0400 Subject: [PATCH 28/42] Use CMAKE_CXX_STANDARD instead of setting CMAKE_CXX_FLAGS. --- c++11/CMakeLists.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/c++11/CMakeLists.txt b/c++11/CMakeLists.txt index 191031c..2657b11 100644 --- a/c++11/CMakeLists.txt +++ b/c++11/CMakeLists.txt @@ -1,4 +1,4 @@ -set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -std=c++11") +set(CMAKE_CXX_STANDARD 11) if ("${CMAKE_CXX_COMPILER_ID}" STREQUAL "Clang") set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -Weverything \ -Wno-c++98-compat \ From cd1f27b00ebab7285f3b23e3f4785472517792e7 Mon Sep 17 00:00:00 2001 From: Ryan Burn Date: Fri, 16 Jun 2017 18:13:50 -0400 Subject: [PATCH 29/42] Set up clang-tidy to run with build. --- c++11/CMakeLists.txt | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/c++11/CMakeLists.txt b/c++11/CMakeLists.txt index 2657b11..522b0cf 100644 --- a/c++11/CMakeLists.txt +++ b/c++11/CMakeLists.txt @@ -12,6 +12,16 @@ elseif ("${CMAKE_CXX_COMPILER_ID}" STREQUAL "GNU") set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -Wall -Wextra -Wno-unused-parameter") endif() +find_program(CLANG_TIDY_EXE NAMES "clang-tidy" + DOC "Path to clang-tidy executable") +if(NOT CLANG_TIDY_EXE) + message(STATUS "clang-tidy not found.") +else() + message(STATUS "clang-tidy found: ${CLANG_TIDY_EXE}") + set(DO_CLANG_TIDY "${CLANG_TIDY_EXE}" "-checks=*,-clang-analyzer-alpha.*") +endif() + + configure_file(version.h.in include/opentracing/version.h) include_directories(${CMAKE_CURRENT_BINARY_DIR}/include) install(DIRECTORY ${CMAKE_CURRENT_BINARY_DIR}/include/opentracing @@ -30,6 +40,10 @@ set(SRCS src/propagation.cpp src/noop.cpp src/tracer.cpp) add_library(opentracing SHARED ${SRCS}) add_library(opentracing-static STATIC ${SRCS}) set_target_properties(opentracing-static PROPERTIES OUTPUT_NAME opentracing) +if (CLANG_TIDY_EXE) + set_target_properties(opentracing PROPERTIES + CXX_CLANG_TIDY "${DO_CLANG_TIDY}") +endif() install(TARGETS opentracing opentracing-static LIBRARY DESTINATION lib ARCHIVE DESTINATION lib) From 5dc5af02c9436ebe48996d90c55e345575e02dd9 Mon Sep 17 00:00:00 2001 From: Ryan Burn Date: Fri, 16 Jun 2017 19:32:45 -0400 Subject: [PATCH 30/42] Use macros for opening and closing the inline namespace. --- c++11/include/opentracing/noop.h | 4 ++-- c++11/include/opentracing/propagation.h | 4 ++-- c++11/include/opentracing/span.h | 4 ++-- c++11/include/opentracing/stringref.h | 5 ++--- c++11/include/opentracing/tracer.h | 4 ++-- c++11/include/opentracing/util.h | 4 ++-- c++11/include/opentracing/value.h | 4 ++-- c++11/src/noop.cpp | 4 ++-- c++11/src/propagation.cpp | 4 ++-- c++11/src/tracer.cpp | 4 ++-- c++11/version.h.in | 9 +++++++-- 11 files changed, 27 insertions(+), 23 deletions(-) diff --git a/c++11/include/opentracing/noop.h b/c++11/include/opentracing/noop.h index 58b806b..7b2e18d 100644 --- a/c++11/include/opentracing/noop.h +++ b/c++11/include/opentracing/noop.h @@ -6,7 +6,7 @@ #include namespace opentracing { -inline namespace OPENTRACING_INLINE_NAMESPACE { +BEGIN_OPENTRACING_ABI_NAMESPACE // A NoopTracer is a trivial, minimum overhead implementation of Tracer // for which all operations are no-ops. // @@ -21,7 +21,7 @@ inline namespace OPENTRACING_INLINE_NAMESPACE { // // WARNING: NoopTracer does not support baggage propagation. std::shared_ptr make_noop_tracer() noexcept; -} // namespace OPENTRACING_INLINE_NAMESPACE +END_OPENTRACING_ABI_NAMESPACE } // namespace opentracing #endif // OPENTRACING_NOOP_H diff --git a/c++11/include/opentracing/propagation.h b/c++11/include/opentracing/propagation.h index 3e97826..3b9a15e 100644 --- a/c++11/include/opentracing/propagation.h +++ b/c++11/include/opentracing/propagation.h @@ -9,7 +9,7 @@ #include namespace opentracing { -inline namespace OPENTRACING_INLINE_NAMESPACE { +BEGIN_OPENTRACING_ABI_NAMESPACE enum class SpanReferenceType { // ChildOfRef refers to a parent Span that caused *and* somehow depends // upon the new child Span. Often (but not always), the parent Span cannot @@ -187,7 +187,7 @@ class HTTPHeadersReader : public TextMapReader {}; // With it, the caller can encode a SpanContext for propagation as entries in // http request headers class HTTPHeadersWriter : public TextMapWriter {}; -} // namespace OPENTRACING_INLINE_NAMESPACE +END_OPENTRACING_ABI_NAMESPACE } // namespace opentracing #endif // OPENTRACING_PROPAGATION_H diff --git a/c++11/include/opentracing/span.h b/c++11/include/opentracing/span.h index f18bc57..54ca27a 100644 --- a/c++11/include/opentracing/span.h +++ b/c++11/include/opentracing/span.h @@ -10,7 +10,7 @@ #include namespace opentracing { -inline namespace OPENTRACING_INLINE_NAMESPACE { +BEGIN_OPENTRACING_ABI_NAMESPACE class Tracer; // SpanContext represents Span state that must propagate to descendant Spans and @@ -162,7 +162,7 @@ class FinishTimestamp : public FinishSpanOption { private: SteadyTime steady_when_; }; -} // namespace OPENTRACING_INLINE_NAMESPACE +END_OPENTRACING_ABI_NAMESPACE } // namespace opentracing #endif // OPENTRACING_SPAN_H diff --git a/c++11/include/opentracing/stringref.h b/c++11/include/opentracing/stringref.h index 541262a..0443db9 100644 --- a/c++11/include/opentracing/stringref.h +++ b/c++11/include/opentracing/stringref.h @@ -37,7 +37,7 @@ // sudden. That design issue still needs to be addressed. namespace opentracing { -inline namespace OPENTRACING_INLINE_NAMESPACE { +BEGIN_OPENTRACING_ABI_NAMESPACE // =============== // class StringRef // =============== @@ -167,8 +167,7 @@ inline std::ostream& operator<<(std::ostream& os, const opentracing::StringRef& ref) { return os.write(ref.data(), static_cast(ref.length())); } - -} // namespace OPENTRACING_INLINE_NAMESPACE +END_OPENTRACING_ABI_NAMESPACE } // namespace opentracing #endif // OPENTRACING_STRINGREF_H diff --git a/c++11/include/opentracing/tracer.h b/c++11/include/opentracing/tracer.h index 201f194..1fb1a15 100644 --- a/c++11/include/opentracing/tracer.h +++ b/c++11/include/opentracing/tracer.h @@ -13,7 +13,7 @@ #include namespace opentracing { -inline namespace OPENTRACING_INLINE_NAMESPACE { +BEGIN_OPENTRACING_ABI_NAMESPACE // StartSpanOptions allows Tracer.StartSpan() callers a mechanism to override // the start timestamp, specify Span References, and make a single Tag or // multiple Tags available at Span start time. @@ -226,7 +226,7 @@ class SetTag : public StartSpanOption { StringRef key_; const Value& value_; }; -} // namespace OPENTRACING_INLINE_NAMESPACE +END_OPENTRACING_ABI_NAMESPACE } // namespace opentracing #endif // OPENTRACING_TRACER_H diff --git a/c++11/include/opentracing/util.h b/c++11/include/opentracing/util.h index 4073078..28d7263 100644 --- a/c++11/include/opentracing/util.h +++ b/c++11/include/opentracing/util.h @@ -7,7 +7,7 @@ #include namespace opentracing { -inline namespace OPENTRACING_INLINE_NAMESPACE { +BEGIN_OPENTRACING_ABI_NAMESPACE using SystemClock = std::chrono::system_clock; using SteadyClock = std::chrono::steady_clock; using SystemTime = SystemClock::time_point; @@ -52,7 +52,7 @@ typename ToClock::time_point convert_time_point( return to_now + std::chrono::duration_cast( from_time_point - from_now); } -} // namespace OPENTRACING_INLINE_NAMESPACE +END_OPENTRACING_ABI_NAMESPACE } // namespace opentracing #endif // OPENTRACING_UTIL_H diff --git a/c++11/include/opentracing/value.h b/c++11/include/opentracing/value.h index 1df12d7..02a9c7b 100644 --- a/c++11/include/opentracing/value.h +++ b/c++11/include/opentracing/value.h @@ -10,7 +10,7 @@ #include namespace opentracing { -inline namespace OPENTRACING_INLINE_NAMESPACE { +BEGIN_OPENTRACING_ABI_NAMESPACE // Variant value types for span tags and log payloads. class Value; @@ -63,7 +63,7 @@ class Value : public variant_type { Value(const Dictionary& values) : variant_type(values) {} Value(Dictionary&& values) : variant_type(std::move(values)) {} }; -} // namespace OPENTRACING_INLINE_NAMESPACE +END_OPENTRACING_ABI_NAMESPACE } // namespace opentracing #endif // OPENTRACING_VALUE_H diff --git a/c++11/src/noop.cpp b/c++11/src/noop.cpp index 78ed4fa..70e72d5 100644 --- a/c++11/src/noop.cpp +++ b/c++11/src/noop.cpp @@ -1,7 +1,7 @@ #include namespace opentracing { -inline namespace OPENTRACING_INLINE_NAMESPACE { +BEGIN_OPENTRACING_ABI_NAMESPACE namespace { class NoopSpanContext : public SpanContext { public: @@ -58,5 +58,5 @@ class NoopTracer : public Tracer, std::shared_ptr make_noop_tracer() noexcept { return std::shared_ptr(new (std::nothrow) NoopTracer()); } -} // namespace OPENTRACING_INLINE_NAMESPACE +END_OPENTRACING_ABI_NAMESPACE } // namesapce opentracing diff --git a/c++11/src/propagation.cpp b/c++11/src/propagation.cpp index fb71253..3b77c27 100644 --- a/c++11/src/propagation.cpp +++ b/c++11/src/propagation.cpp @@ -1,7 +1,7 @@ #include namespace opentracing { -inline namespace OPENTRACING_INLINE_NAMESPACE { +BEGIN_OPENTRACING_ABI_NAMESPACE namespace { class PropagationErrorCategory : public std::error_category { public: @@ -42,5 +42,5 @@ const std::error_category& propagation_error_category() { static const PropagationErrorCategory error_category; return error_category; } -} // namespace OPENTRACING_INLINE_NAMESPACE +END_OPENTRACING_ABI_NAMESPACE } // namespace opentracing diff --git a/c++11/src/tracer.cpp b/c++11/src/tracer.cpp index 17c47ac..f16c73e 100644 --- a/c++11/src/tracer.cpp +++ b/c++11/src/tracer.cpp @@ -2,7 +2,7 @@ #include namespace opentracing { -inline namespace OPENTRACING_INLINE_NAMESPACE { +BEGIN_OPENTRACING_ABI_NAMESPACE static std::shared_ptr& get_global_tracer() { static std::shared_ptr global_tracer = make_noop_tracer(); return global_tracer; @@ -17,5 +17,5 @@ std::shared_ptr Tracer::InitGlobal( get_global_tracer().swap(tracer); return tracer; } -} // namespace OPENTRACING_INLINE_NAMESPACE +END_OPENTRACING_ABI_NAMESPACE } // namespace opentracing diff --git a/c++11/version.h.in b/c++11/version.h.in index a407253..bd0bc0e 100644 --- a/c++11/version.h.in +++ b/c++11/version.h.in @@ -1,10 +1,15 @@ #ifndef OPENTRACING_VERSION_H #define OPENTRACING_VERSION_H -#define OPENTRACING_INLINE_NAMESPACE v${OPENTRACING_ABI_VERSION} - #define OPENTRACING_VERSION "${CPACK_PACKAGE_VERSION_MAJOR}." \ "${CPACK_PACKAGE_VERSION_MINOR}." \ "${CPACK_PACKAGE_VERSION_PATCH}" +// clang-format off +#define BEGIN_OPENTRACING_ABI_NAMESPACE \ + inline namespace v${OPENTRACING_ABI_VERSION} { +#define END_OPENTRACING_ABI_NAMESPACE \ + } // namespace v${OPENTRACING_ABI_VERSION} +// clang-format on + #endif // OPENTRACING_VERSION_H From 042c15c99a0ed1f1c3f70d2f81defda434fd61a0 Mon Sep 17 00:00:00 2001 From: Ryan Burn Date: Fri, 16 Jun 2017 19:34:59 -0400 Subject: [PATCH 31/42] Fix namespace comment. --- c++11/src/noop.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/c++11/src/noop.cpp b/c++11/src/noop.cpp index 70e72d5..dd21263 100644 --- a/c++11/src/noop.cpp +++ b/c++11/src/noop.cpp @@ -59,4 +59,4 @@ std::shared_ptr make_noop_tracer() noexcept { return std::shared_ptr(new (std::nothrow) NoopTracer()); } END_OPENTRACING_ABI_NAMESPACE -} // namesapce opentracing +} // namespace opentracing From 4df0a78cfc45ce2bb3c60cf17552a1eecde61131 Mon Sep 17 00:00:00 2001 From: Ryan Burn Date: Fri, 16 Jun 2017 19:49:43 -0400 Subject: [PATCH 32/42] Correct clang-tidy warnings. --- c++11/CMakeLists.txt | 3 +-- c++11/src/noop.cpp | 28 +++++++++++++++------------- c++11/src/propagation.cpp | 30 ++++++++++++++++++------------ 3 files changed, 34 insertions(+), 27 deletions(-) diff --git a/c++11/CMakeLists.txt b/c++11/CMakeLists.txt index 522b0cf..641cc55 100644 --- a/c++11/CMakeLists.txt +++ b/c++11/CMakeLists.txt @@ -3,13 +3,12 @@ if ("${CMAKE_CXX_COMPILER_ID}" STREQUAL "Clang") set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -Weverything \ -Wno-c++98-compat \ -Wno-c++98-compat-bind-to-temporary-copy \ - -Wno-unused-parameter \ -Wno-weak-vtables \ -Wno-exit-time-destructors \ -Wno-global-constructors \ -Wno-padded") elseif ("${CMAKE_CXX_COMPILER_ID}" STREQUAL "GNU") - set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -Wall -Wextra -Wno-unused-parameter") + set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -Wall -Wextra") endif() find_program(CLANG_TIDY_EXE NAMES "clang-tidy" diff --git a/c++11/src/noop.cpp b/c++11/src/noop.cpp index dd21263..bc888ec 100644 --- a/c++11/src/noop.cpp +++ b/c++11/src/noop.cpp @@ -6,8 +6,8 @@ namespace { class NoopSpanContext : public SpanContext { public: void ForeachBaggageItem( - std::function f) - const override {} + std::function /*f*/) const override {} }; class NoopSpan : public Span { @@ -15,16 +15,17 @@ class NoopSpan : public Span { explicit NoopSpan(std::shared_ptr&& tracer) noexcept : tracer_(std::move(tracer)) {} void FinishWithOptions( - const FinishSpanOptions& finish_span_options) noexcept override {} - void SetOperationName(StringRef name) noexcept override {} - void SetTag(StringRef key, const Value& value) noexcept override {} - void SetBaggageItem(StringRef restricted_key, - StringRef value) noexcept override {} - std::string BaggageItem(StringRef restricted_key) const noexcept override { + const FinishSpanOptions& /*finish_span_options*/) noexcept override {} + void SetOperationName(StringRef /*name*/) noexcept override {} + void SetTag(StringRef /*key*/, const Value& /*value*/) noexcept override {} + void SetBaggageItem(StringRef /*restricted_key*/, + StringRef /*value*/) noexcept override {} + std::string BaggageItem(StringRef /*restricted_key*/) const + noexcept override { return {}; } void Log(std::initializer_list> - fields) noexcept override {} + /*fields*/) noexcept override {} const SpanContext& context() const noexcept override { return span_context_; } const Tracer& tracer() const noexcept override { return *tracer_; } @@ -37,19 +38,20 @@ class NoopTracer : public Tracer, public std::enable_shared_from_this { public: std::unique_ptr StartSpanWithOptions( - StringRef operation_name, const StartSpanOptions& options) const + StringRef /*operation_name*/, const StartSpanOptions& /*options*/) const noexcept override { return std::unique_ptr(new (std::nothrow) NoopSpan(shared_from_this())); } - Expected Inject(const SpanContext& sc, CarrierFormat format, - const CarrierWriter& writer) const override { + Expected Inject(const SpanContext& /*sc*/, CarrierFormat /*format*/, + const CarrierWriter& /*writer*/) const override { return {}; } Expected> Extract( - CarrierFormat format, const CarrierReader& reader) const override { + CarrierFormat /*format*/, + const CarrierReader& /*reader*/) const override { return std::unique_ptr(nullptr); } }; diff --git a/c++11/src/propagation.cpp b/c++11/src/propagation.cpp index 3b77c27..a09dc95 100644 --- a/c++11/src/propagation.cpp +++ b/c++11/src/propagation.cpp @@ -11,29 +11,35 @@ class PropagationErrorCategory : public std::error_category { std::error_condition default_error_condition(int code) const noexcept override { - if (code == unsupported_format_error.value()) + if (code == unsupported_format_error.value()) { return std::make_error_condition(std::errc::not_supported); - else if (code == invalid_span_context_error.value()) + } + if (code == invalid_span_context_error.value()) { return std::make_error_condition(std::errc::not_supported); - else if (code == invalid_carrier_error.value()) + } + if (code == invalid_carrier_error.value()) { return std::make_error_condition(std::errc::invalid_argument); - else if (code == span_context_corrupted_error.value()) + } + if (code == span_context_corrupted_error.value()) { return std::make_error_condition(std::errc::invalid_argument); - else - return std::error_condition(code, *this); + } + return std::error_condition(code, *this); } std::string message(int code) const override { - if (code == unsupported_format_error.value()) + if (code == unsupported_format_error.value()) { return "opentracing: Unknown or unsupported Inject/Extract format"; - else if (code == invalid_span_context_error.value()) + } + if (code == invalid_span_context_error.value()) { return "opentracing: SpanContext type incompatible with tracer"; - else if (code == invalid_carrier_error.value()) + } + if (code == invalid_carrier_error.value()) { return "opentracing: Invalid Inject/Extract carrier"; - else if (code == span_context_corrupted_error.value()) + } + if (code == span_context_corrupted_error.value()) { return "opentracing: SpanContext data corrupted in Extract carrier"; - else - return "opentracing: unknown propagation error"; + } + return "opentracing: unknown propagation error"; } }; } // anonymous namespace From 36d3eb481e862824f9db3d3302758b529a416c92 Mon Sep 17 00:00:00 2001 From: Ryan Burn Date: Fri, 16 Jun 2017 19:54:59 -0400 Subject: [PATCH 33/42] s/reset/Reset/g --- c++11/include/opentracing/stringref.h | 12 ++++++------ c++11/test/stringref_test.cpp | 10 +++++----- 2 files changed, 11 insertions(+), 11 deletions(-) diff --git a/c++11/include/opentracing/stringref.h b/c++11/include/opentracing/stringref.h index 0443db9..588f308 100644 --- a/c++11/include/opentracing/stringref.h +++ b/c++11/include/opentracing/stringref.h @@ -78,32 +78,32 @@ class StringRef { // Reset the string reference given a const character array template - void reset(const char (&str)[N]) noexcept { + void Reset(const char (&str)[N]) noexcept { data_ = str; length_ = N; } // Reset this string ref to point at the supplied c-string - void reset(const char* str) noexcept { + void Reset(const char* str) noexcept { data_ = str; length_ = std::strlen(str); } // Reset the string reference given a std::string - void reset(const std::basic_string& str) noexcept { + void Reset(const std::basic_string& str) noexcept { data_ = str.data(); length_ = str.length(); } // Reset this string ref to point at the supplied 'str' of 'length' bytes. - void reset(const char* str, const size_t length) noexcept { + void Reset(const char* str, const size_t length) noexcept { data_ = str; length_ = length; } - // Disallow reset from non-const array + // Disallow Reset from non-const array template - void reset(char (&str)[N]) = delete; + void Reset(char (&str)[N]) = delete; // Return address of the referenced string const char* data() const noexcept { return data_; } diff --git a/c++11/test/stringref_test.cpp b/c++11/test/stringref_test.cpp index 7927fb2..41c9a65 100644 --- a/c++11/test/stringref_test.cpp +++ b/c++11/test/stringref_test.cpp @@ -52,7 +52,7 @@ static void test_reset() { assert(0 == ref.data()); assert(0 == ref.length()); - ref.reset("hello world"); + ref.Reset("hello world"); assert("hello world" == ref); assert(std::string("hello world") == std::string(ref.data())); @@ -60,7 +60,7 @@ static void test_reset() { const char arr[] = "hello world 1"; - ref.reset(arr); + ref.Reset(arr); assert(arr == ref.data()); assert(std::strlen(arr) == ref.length()); assert(std::string(arr) == std::string(ref)); @@ -68,7 +68,7 @@ static void test_reset() { const char* p = "hello world 2"; - ref.reset(p); + ref.Reset(p); assert(p == ref.data()); assert(std::strlen(p) == ref.length()); assert(std::string(p) == std::string(ref)); @@ -76,13 +76,13 @@ static void test_reset() { const std::string s = "hello world 3"; - ref.reset(s); + ref.Reset(s); assert(s.data() == ref.data()); assert(s.length() == ref.length()); assert(std::string(s.c_str()) == std::string(ref)); assert(std::string(s.c_str()) == std::string(ref.data())); - ref.reset(p, std::strlen(p)); + ref.Reset(p, std::strlen(p)); assert(p == ref.data()); assert(std::strlen(p) == ref.length()); assert(p == ref); From 67ef4354d8190d63f62331894980962ed8632312 Mon Sep 17 00:00:00 2001 From: Ryan Burn Date: Sat, 17 Jun 2017 11:44:02 -0400 Subject: [PATCH 34/42] Correction in README.md. --- README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/README.md b/README.md index 5a2c90f..9c1b7ae 100644 --- a/README.md +++ b/README.md @@ -77,7 +77,7 @@ reference. auto tracer = /* Some Tracer */ auto span = tracer->StartSpan( "operation_name", - {opentracing::ChildOf(parent_span.context())}); + {opentracing::ChildOf(&parent_span.context())}); if (!span) // Error creating span. ... From c98410b1e9a44ebcc7042d168961c28e6825787d Mon Sep 17 00:00:00 2001 From: Ryan Burn Date: Sat, 17 Jun 2017 11:51:03 -0400 Subject: [PATCH 35/42] Update 3rd party expected library. --- .../martinmoene_expected/expected.hpp | 174 +++++++++--------- 1 file changed, 92 insertions(+), 82 deletions(-) diff --git a/c++11/3rd_party/include/opentracing/martinmoene_expected/expected.hpp b/c++11/3rd_party/include/opentracing/martinmoene_expected/expected.hpp index ce99429..62ae853 100644 --- a/c++11/3rd_party/include/opentracing/martinmoene_expected/expected.hpp +++ b/c++11/3rd_party/include/opentracing/martinmoene_expected/expected.hpp @@ -300,12 +300,12 @@ constexpr bool operator>=( unexpected_type const & x, unexpected_type cons return ! ( x < y ); } -inline constexpr bool operator<( unexpected_type const & x, unexpected_type const & y ) +inline constexpr bool operator<( unexpected_type const & /*x*/, unexpected_type const & /*y*/ ) { return false; } -inline constexpr bool operator>( unexpected_type const & x, unexpected_type const & y ) +inline constexpr bool operator>( unexpected_type const & /*x*/, unexpected_type const & /*y*/ ) { return false; } @@ -401,7 +401,7 @@ class expected ( std::is_nothrow_default_constructible::value ) - : has_value( true ) + : has_value_( true ) { contained.construct_value( value_type() ); } @@ -411,10 +411,10 @@ class expected // std::is_copy_constructible::value ) nsel_constexpr14 expected( expected const & rhs ) - : has_value( rhs.has_value ) + : has_value_( rhs.has_value_ ) { - if ( has_value ) contained.construct_value( rhs.contained.value() ); - else contained.construct_error( rhs.contained.error() ); + if ( has_value() ) contained.construct_value( rhs.contained.value() ); + else contained.construct_error( rhs.contained.error() ); } // nsel_REQUIRES_0( @@ -422,17 +422,17 @@ class expected // std::is_move_constructible::value ) nsel_constexpr14 expected( expected && rhs ) - : has_value( rhs.has_value ) + : has_value_( rhs.has_value_ ) { - if ( has_value ) contained.construct_value( std::move( rhs.contained.value() ) ); - else contained.construct_error( std::move( rhs.contained.error() ) ); + if ( has_value() ) contained.construct_value( std::move( rhs.contained.value() ) ); + else contained.construct_error( std::move( rhs.contained.error() ) ); } nsel_REQUIRES_0( std::is_copy_constructible::value ) nsel_constexpr14 expected( value_type const & rhs ) - : has_value( true ) + : has_value_( true ) { contained.construct_value( rhs ); } @@ -443,9 +443,9 @@ class expected nsel_constexpr14 expected( value_type && rhs ) noexcept ( std::is_nothrow_move_constructible::value && - std::is_nothrow_move_constructible::value + std::is_nothrow_move_constructible::value ) - : has_value( true ) + : has_value_( true ) { contained.construct_value( std::move( rhs ) ); } @@ -454,7 +454,7 @@ class expected std::is_constructible::value ) > nsel_constexpr14 explicit expected( in_place_t, Args&&... args ) - : has_value( true ) + : has_value_( true ) { contained.construct_value( std::forward( args )... ); } @@ -463,7 +463,7 @@ class expected std::is_constructible, Args&&...>::value ) > nsel_constexpr14 explicit expected( in_place_t, std::initializer_list il, Args&&... args ) - : has_value( true ) + : has_value_( true ) { contained.construct_value( il, std::forward( args )... ); } @@ -472,7 +472,7 @@ class expected std::is_copy_constructible::value ) nsel_constexpr14 expected( unexpected_type const & error ) - : has_value( false ) + : has_value_( false ) { contained.construct_error( error.value() ); } @@ -481,7 +481,7 @@ class expected std::is_move_constructible::value ) nsel_constexpr14 expected( unexpected_type && error ) - : has_value( false ) + : has_value_( false ) { contained.construct_error( std::move( error.value() ) ); } @@ -490,7 +490,7 @@ class expected std::is_constructible::value ) > nsel_constexpr14 explicit expected( in_place_unexpected_t, Args&&... args ) - : has_value( false ) + : has_value_( false ) { contained.construct_error( std::forward( args )... ); } @@ -499,7 +499,7 @@ class expected std::is_constructible, Args&&...>::value ) > nsel_constexpr14 explicit expected( in_place_unexpected_t, std::initializer_list il, Args&&... args ) - : has_value( false ) + : has_value_( false ) { contained.construct_error( il, std::forward( args )... ); } @@ -508,8 +508,8 @@ class expected ~expected() { - if ( has_value ) contained.destruct_value(); - else contained.destruct_error(); + if ( has_value() ) contained.destruct_value(); + else contained.destruct_error(); } // assignment @@ -530,7 +530,7 @@ class expected // std::is_move_constructible::value && // std::is_move_assignable::value && // std::is_move_constructible::value && -// std::is_move_assignable::value ) +// std::is_move_assignable::value ) expected & operator=( expected && rhs ) noexcept ( @@ -549,7 +549,7 @@ class expected expected & operator=( U && v ) { - expected( std::forward( v ) ).swap( *this ); + expected( std::forward( v ) ).swap( *this ); return *this; } @@ -559,7 +559,7 @@ class expected expected & operator=( unexpected_type const & u ) { - expected( std::move( u ) ).swap( *this ); + expected( std::move( u ) ).swap( *this ); return *this; } @@ -569,7 +569,7 @@ class expected expected & operator=( unexpected_type && u ) { - expected( std::move( u ) ).swap( *this ); + expected( std::move( u ) ).swap( *this ); return *this; } @@ -594,7 +594,7 @@ class expected // std::is_move_constructible::value ) void swap( expected & rhs ) noexcept - ( + ( std::is_nothrow_move_constructible::value && noexcept( std::swap( std::declval(), std::declval() ) ) && std::is_nothrow_move_constructible::value && noexcept( std::swap( std::declval(), std::declval() ) ) ) { @@ -602,12 +602,12 @@ class expected if ( bool(*this) && bool(rhs) ) { swap( contained.value(), rhs.contained.value() ); } else if ( ! bool(*this) && ! bool(rhs) ) { swap( contained.error(), rhs.contained.error() ); } - else if ( bool(*this) && ! bool(rhs) ) { error_type t( std::move( rhs.error() ) ); + else if ( bool(*this) && ! bool(rhs) ) { error_type t( std::move( rhs.error() ) ); // TBD - ?? rhs.contained.destruct_error(); rhs.contained.construct_value( std::move( contained.value() ) ); // TBD - ?? contained.destruct_value(); contained.construct_error( std::move( t ) ); - swap( has_value, rhs.has_value ); } + swap( has_value_, rhs.has_value_ ); } else if ( ! bool(*this) && bool(rhs) ) { rhs.swap( *this ); } } @@ -615,37 +615,42 @@ class expected constexpr value_type const * operator ->() const { - return assert( has_value ), contained.value_ptr(); + return assert( has_value() ), contained.value_ptr(); } value_type * operator ->() { - return assert( has_value ), contained.value_ptr(); + return assert( has_value() ), contained.value_ptr(); } constexpr value_type const & operator *() const & { - return assert( has_value ), contained.value(); + return assert( has_value() ), contained.value(); } value_type & operator *() & { - return assert( has_value ), contained.value(); + return assert( has_value() ), contained.value(); } constexpr value_type && operator *() && { - return assert( has_value ), std::move( contained.value() ); + return assert( has_value() ), std::move( contained.value() ); } constexpr explicit operator bool() const noexcept { - return has_value; + return has_value(); + } + + constexpr bool has_value() const noexcept + { + return has_value_; } constexpr value_type const & value() const & { - return has_value + return has_value() ? contained.value() : std::is_same::value ? ( std::rethrow_exception( contained.error() ), contained.value() ) @@ -654,7 +659,7 @@ class expected value_type & value() & { - return has_value + return has_value() ? contained.value() : std::is_same::value ? ( std::rethrow_exception( contained.error() ), contained.value() ) @@ -663,7 +668,7 @@ class expected constexpr value_type && value() && { - return has_value + return has_value() ? std::move( contained.value() ) : std::is_same::value ? ( std::rethrow_exception( contained.error() ), contained.value() ) @@ -672,17 +677,17 @@ class expected constexpr error_type const & error() const & { - return assert( ! has_value ), contained.error(); + return assert( ! has_value() ), contained.error(); } error_type & error() & { - return assert( ! has_value ), contained.error(); + return assert( ! has_value() ), contained.error(); } constexpr error_type && error() && { - return assert( ! has_value ), std::move( contained.error() ); + return assert( ! has_value() ), std::move( contained.error() ); } constexpr unexpected_type get_unexpected() const @@ -693,7 +698,7 @@ class expected template< typename Ex > bool has_exception() const { - return ! has_value && std::is_base_of< Ex, decltype( get_unexpected().value() ) >::value; + return ! has_value() && std::is_base_of< Ex, decltype( get_unexpected().value() ) >::value; } template< typename U, nsel_REQUIRES_T( @@ -702,19 +707,19 @@ class expected value_type value_or( U && v ) const & { - return has_value - ? contained.value() + return has_value() + ? contained.value() : static_cast( std::forward( v ) ); } template< typename U, nsel_REQUIRES_T( std::is_move_constructible::value && std::is_convertible::value ) > - + value_type value_or( U && v ) && { - return has_value - ? std::move( contained.value() ) + return has_value() + ? std::move( contained.value() ) : static_cast( std::forward( v ) ); } @@ -750,7 +755,7 @@ class expected // ’see below’ then(F&& func); private: - bool has_value; + bool has_value_; expected_detail::storage_t contained; }; @@ -759,30 +764,30 @@ class expected template< typename E > class expected { -public: +public: typedef void value_type; typedef E error_type; - + template< typename U > - struct rebind + struct rebind { typedef expected type; }; - + // constructors constexpr expected() noexcept - : has_value( true ) - { + : has_value_( true ) + { } // nsel_REQUIRES_0( // std::is_copy_constructible::value ) nsel_constexpr14 expected( expected const & rhs ) - : has_value( rhs.has_value ) - { - if ( ! has_value ) contained.construct_error( rhs.contained.error() ); + : has_value_( rhs.has_value_ ) + { + if ( ! has_value() ) contained.construct_error( rhs.contained.error() ); } nsel_REQUIRES_0( @@ -792,17 +797,17 @@ class expected ( true // TBD - see also non-void specialization ) - : has_value( rhs.has_value ) - { - if ( ! has_value ) contained.construct_error( std::move( rhs.contained.error() ) ); + : has_value_( rhs.has_value_ ) + { + if ( ! has_value() ) contained.construct_error( std::move( rhs.contained.error() ) ); } //nsel_REQUIRES_0( // std::is_default_constructible::value ) - constexpr explicit expected( in_place_t ) - : has_value( true ) - { + constexpr explicit expected( in_place_t ) + : has_value_( true ) + { } // nsel_REQUIRES( @@ -810,16 +815,16 @@ class expected // std::is_assignable::value ) nsel_constexpr14 expected( unexpected_type const & error ) - : has_value( false ) + : has_value_( false ) { contained.construct_error( error.value() ); } // ?? expected( unexpected_type && error ) - + template nsel_constexpr14 expected( unexpected_type const & error ) - : has_value( false ) + : has_value_( false ) { contained.construct_error( error.value() ); } @@ -828,7 +833,7 @@ class expected ~expected() { - if ( ! has_value ) contained.destruct_error(); + if ( ! has_value() ) contained.destruct_error(); } // assignment @@ -845,7 +850,7 @@ class expected // nsel_REQUIRES( // std::is_move_constructible::value && -// std::is_move_assignable::value ) +// std::is_move_assignable::value ) expected & operator=( expected && rhs ) noexcept ( @@ -856,7 +861,7 @@ class expected return *this; } - void emplace() + void emplace() {} // swap @@ -873,7 +878,7 @@ class expected if ( ! bool(*this) && ! bool(rhs) ) { swap( contained.error(), rhs.contained.error() ); } else if ( bool(*this) && ! bool(rhs) ) { contained.construct_error( std::move( rhs.error() ) ); - swap( has_value, rhs.has_value ); } + swap( has_value_, rhs.has_value_ ); } else if ( ! bool(*this) && bool(rhs) ) { rhs.swap( *this ); } } @@ -881,7 +886,12 @@ class expected constexpr explicit operator bool() const noexcept { - return has_value; + return has_value(); + } + + constexpr bool has_value() const noexcept + { + return has_value_; } void value() const @@ -889,17 +899,17 @@ class expected constexpr error_type const & error() const & { - return assert( ! has_value ), contained.error(); + return assert( ! has_value() ), contained.error(); } error_type & error() & { - return assert( ! has_value ), contained.error(); + return assert( ! has_value() ), contained.error(); } constexpr error_type && error() && { - return assert( ! has_value ), std::move( contained.error() ); + return assert( ! has_value() ), std::move( contained.error() ); } constexpr unexpected_type get_unexpected() const @@ -910,7 +920,7 @@ class expected template bool has_exception() const { - return ! has_value && std::is_base_of< Ex, decltype( get_unexpected().value() ) >::value; + return ! has_value() && std::is_base_of< Ex, decltype( get_unexpected().value() ) >::value; } // template constexpr ’see below’ unwrap() const&; @@ -933,9 +943,9 @@ class expected // // template // ’see below’ then(F&& func); - + private: - bool has_value; + bool has_value_; expected_detail::storage_t contained; }; @@ -1165,9 +1175,9 @@ constexpr auto make_expected_from_error( E e ) -> expected -/*nsel_constexpr14*/ +/*nsel_constexpr14*/ auto make_expected_from_call( F f, - nsel_REQUIRES( ! std::is_same::type, void>::value ) + nsel_REQUIRES( ! std::is_same::type, void>::value ) ) -> expected< typename std::result_of::type > { try @@ -1181,9 +1191,9 @@ auto make_expected_from_call( F f, } template< typename F > -/*nsel_constexpr14*/ -auto make_expected_from_call( F f, - nsel_REQUIRES( std::is_same::type, void>::value ) +/*nsel_constexpr14*/ +auto make_expected_from_call( F f, + nsel_REQUIRES( std::is_same::type, void>::value ) ) -> expected { try @@ -1229,8 +1239,8 @@ struct hash< nonstd::expected > }; // TBD - implement -// bool(e), hash>()(e) shall evaluate to the hashing true; -// otherwise it evaluates to an unspecified value if E is exception_ptr or +// bool(e), hash>()(e) shall evaluate to the hashing true; +// otherwise it evaluates to an unspecified value if E is exception_ptr or // a combination of hashing false and hash()(e.error()). template< typename E > From 65f6978d23073c3663d74431be5dec9d8243c39e Mon Sep 17 00:00:00 2001 From: Ryan Burn Date: Sat, 17 Jun 2017 12:04:18 -0400 Subject: [PATCH 36/42] Add a test for convert_time_point. --- c++11/test/CMakeLists.txt | 3 +++ c++11/test/util_test.cpp | 21 +++++++++++++++++++++ 2 files changed, 24 insertions(+) create mode 100644 c++11/test/util_test.cpp diff --git a/c++11/test/CMakeLists.txt b/c++11/test/CMakeLists.txt index b37379a..a8c76bf 100644 --- a/c++11/test/CMakeLists.txt +++ b/c++11/test/CMakeLists.txt @@ -7,3 +7,6 @@ add_test(stringref_test stringref_test) add_executable(value_test value_test.cpp) add_test(value_test value_test) + +add_executable(util_test util_test.cpp) +add_test(util_test util_test) diff --git a/c++11/test/util_test.cpp b/c++11/test/util_test.cpp new file mode 100644 index 0000000..1874f0b --- /dev/null +++ b/c++11/test/util_test.cpp @@ -0,0 +1,21 @@ +#include +#include +#include +using namespace opentracing; + +static void test_convert_time_point() { + // Check that converting from a system_clock time_point to a steady_clock + // time point and then back again produces approximately the same system_clock + // time_point. + auto t1 = SystemClock::now(); + auto t2 = convert_time_point(t1); + auto t3 = convert_time_point(t2); + auto difference = std::abs( + std::chrono::duration_cast(t3 - t1).count()); + assert(difference < 100); +} + +int main() { + test_convert_time_point(); + return 0; +} From 1346d0a4db63528a146dfe6b7560f4c08d027bc3 Mon Sep 17 00:00:00 2001 From: Ryan Burn Date: Sat, 17 Jun 2017 12:19:34 -0400 Subject: [PATCH 37/42] Check for null in noop_test. --- c++11/test/noop_test.cpp | 2 ++ 1 file changed, 2 insertions(+) diff --git a/c++11/test/noop_test.cpp b/c++11/test/noop_test.cpp index 67dfa91..1f84d23 100644 --- a/c++11/test/noop_test.cpp +++ b/c++11/test/noop_test.cpp @@ -7,9 +7,11 @@ int main() { auto tracer = make_noop_tracer(); auto span1 = tracer->StartSpan("a"); + assert(span1); assert(&span1->tracer() == tracer.get()); auto span2 = tracer->StartSpan("b", {ChildOf(&span1->context())}); + assert(span2); span2->SetOperationName("b1"); span2->SetTag("x", true); assert(span2->BaggageItem("y").empty()); From 19a73e2a4ff7e44787b8b4da42dfe78ad88efdf5 Mon Sep 17 00:00:00 2001 From: Ryan Burn Date: Sun, 18 Jun 2017 13:08:51 -0400 Subject: [PATCH 38/42] Added test for StartSpanOptions. --- c++11/test/CMakeLists.txt | 6 +++--- c++11/test/{noop_test.cpp => tracer_test.cpp} | 14 +++++++++++++- 2 files changed, 16 insertions(+), 4 deletions(-) rename c++11/test/{noop_test.cpp => tracer_test.cpp} (64%) diff --git a/c++11/test/CMakeLists.txt b/c++11/test/CMakeLists.txt index a8c76bf..2542846 100644 --- a/c++11/test/CMakeLists.txt +++ b/c++11/test/CMakeLists.txt @@ -1,6 +1,6 @@ -add_executable(noop_test noop_test.cpp) -target_link_libraries(noop_test opentracing) -add_test(noop_test noop_test) +add_executable(tracer_test tracer_test.cpp) +target_link_libraries(tracer_test opentracing) +add_test(tracer_test tracer_test) add_executable(stringref_test stringref_test.cpp) add_test(stringref_test stringref_test) diff --git a/c++11/test/noop_test.cpp b/c++11/test/tracer_test.cpp similarity index 64% rename from c++11/test/noop_test.cpp rename to c++11/test/tracer_test.cpp index 1f84d23..82ad45a 100644 --- a/c++11/test/noop_test.cpp +++ b/c++11/test/tracer_test.cpp @@ -3,7 +3,7 @@ #include using namespace opentracing; -int main() { +static void test_tracer_interface() { auto tracer = make_noop_tracer(); auto span1 = tracer->StartSpan("a"); @@ -17,6 +17,18 @@ int main() { assert(span2->BaggageItem("y").empty()); span2->Log({{"event", "xyz"}, {"abc", 123}}); span2->Finish(); +} + +static void test_start_span_options() { + StartSpanOptions options; + // A reference to null a SpanContext is ignored. + ChildOf(nullptr).Apply(options); + assert(options.references.size() == 0); +} + +int main() { + test_tracer_interface(); + test_start_span_options(); return 0; } From a5654fcf0ed439ebaacf62f2cbd86c4bf92265d8 Mon Sep 17 00:00:00 2001 From: Ryan Burn Date: Wed, 21 Jun 2017 12:21:40 -0400 Subject: [PATCH 39/42] Corrected the propagation examples. --- c++11/include/opentracing/propagation.h | 23 +++++++++++++++-------- 1 file changed, 15 insertions(+), 8 deletions(-) diff --git a/c++11/include/opentracing/propagation.h b/c++11/include/opentracing/propagation.h index 3b9a15e..323c290 100644 --- a/c++11/include/opentracing/propagation.h +++ b/c++11/include/opentracing/propagation.h @@ -71,18 +71,25 @@ enum class CarrierFormat { // // For example, Inject(): // - // std::vector> *headers = ...; - // if (!span.tracer().Inject(span, CarrierFormat::HTTPHeadersCarrier, - // make_ordered_string_pairs_writer(headers))) { - // throw error("inject failed"); + // const HTTPHeadersReader& carrier_reader = /* some carrier */ + // auto was_successful = span.tracer().Inject(span, + // CarrierFormat::HTTPHeaders, + // carrier_reader); + // if (!was_successful) { + // throw std::runtime_error(was_successful.error().message()); // } // // Or Extract(): // - // SpanContext extracted; - // extracted = Tracer::Global().Extract(CarrierFormat::HTTPHeadersCarrier, - // make_ordered_string_pairs_reader(*headers)); - // auto span = Tracer::Global().StartSpan("op", { ChildOf(extracted) }); + // const Tracer& tracer = /* some tracer */ + // const HTTPHeadersWriter& carrier_writer = /* some carrier */ + // auto span_context_maybe = tracer.Extract(CarrierFormat::HTTPHeaders, + // carrier_writer); + // if (!span_context_maybe) { + // throw std::runtime_error(span_context_maybe.error().message()); + // } + // auto span = tracer.StartSpan("op", + // { ChildOf(span_context_maybe->get()) }); // HTTPHeaders = 2, From 0b7eaec5ca3f3f52fbd3266beca254afc42ed065 Mon Sep 17 00:00:00 2001 From: Ryan Burn Date: Thu, 22 Jun 2017 17:15:09 -0400 Subject: [PATCH 40/42] Removed the argument from Inject/Extract. --- c++11/include/opentracing/propagation.h | 129 +++++++++++------------- c++11/include/opentracing/tracer.h | 33 ++++-- c++11/src/noop.cpp | 16 ++- 3 files changed, 95 insertions(+), 83 deletions(-) diff --git a/c++11/include/opentracing/propagation.h b/c++11/include/opentracing/propagation.h index 323c290..ed06c00 100644 --- a/c++11/include/opentracing/propagation.h +++ b/c++11/include/opentracing/propagation.h @@ -5,11 +5,15 @@ #include #include #include +#include #include #include namespace opentracing { BEGIN_OPENTRACING_ABI_NAMESPACE +class Tracer; +class SpanContext; + enum class SpanReferenceType { // ChildOfRef refers to a parent Span that caused *and* somehow depends // upon the new child Span. Often (but not always), the parent Span cannot @@ -52,60 +56,6 @@ enum class SpanReferenceType { FollowsFromRef = 2 }; -// Carrier format values. -enum class CarrierFormat { - // OpenTracingBinary encodes the SpanContext for propagation as opaque - // binary data. - OpenTracingBinary = 1, // RESERVED, NOT IMPLEMENTED - - // HTTPHeaders represents SpanContexts as HTTP header string pairs. - // - // The HTTPHeaders format requires that the keys and values be valid - // as HTTP headers as-is (i.e., character casing may be unstable and - // special characters are disallowed in keys, values should be - // URL-escaped, etc). - // - // For Tracer::Inject(): the carrier must be a `TextMapReader`. - // - // For Tracer::Extract(): the carrier must be a `TextMapWriter`. - // - // For example, Inject(): - // - // const HTTPHeadersReader& carrier_reader = /* some carrier */ - // auto was_successful = span.tracer().Inject(span, - // CarrierFormat::HTTPHeaders, - // carrier_reader); - // if (!was_successful) { - // throw std::runtime_error(was_successful.error().message()); - // } - // - // Or Extract(): - // - // const Tracer& tracer = /* some tracer */ - // const HTTPHeadersWriter& carrier_writer = /* some carrier */ - // auto span_context_maybe = tracer.Extract(CarrierFormat::HTTPHeaders, - // carrier_writer); - // if (!span_context_maybe) { - // throw std::runtime_error(span_context_maybe.error().message()); - // } - // auto span = tracer.StartSpan("op", - // { ChildOf(span_context_maybe->get()) }); - // - HTTPHeaders = 2, - - // TextMap encodes the SpanContext as key:value pairs. - // - // The TextMap format is similar to the HTTPHeaderes format, - // without restrictions on the character set. - // - // For Tracer::Inject(): the carrier must be a `TextMapReader`. - // - // For Tracer::Extract(): the carrier must be a `TextMapWriter`. - // - // See the HTTPHeaders examples. - TextMap = 3 -}; - // Returns the std::error_category class used for opentracing propagation // errors. // @@ -135,24 +85,15 @@ const std::error_code invalid_carrier_error(2, propagation_error_category()); const std::error_code span_context_corrupted_error( 3, propagation_error_category()); -// Base class for implementation-dependent Tracer::Inject carrier-type -// adapter. -class CarrierReader { - public: - virtual ~CarrierReader() = default; -}; - -// Base class for implementation-dependent Tracer::Extract carrier-type adapter. -class CarrierWriter { - public: - virtual ~CarrierWriter() = default; -}; - // TextMapWriter is the Inject() carrier for the TextMap builtin format. With // it, the caller can encode a SpanContext for propagation as entries in a map // of unicode strings. -class TextMapReader : public CarrierReader { +// +// See the HTTPHeaders examples. +class TextMapReader { public: + virtual ~TextMapReader() = default; + // ForeachKey returns TextMap contents via repeated calls to the `f` // function. If any call to `f` returns an error, ForeachKey terminates and // returns that error. @@ -172,8 +113,12 @@ class TextMapReader : public CarrierReader { // TextMapWriter is the Inject() carrier for the TextMap builtin format. With // it, the caller can encode a SpanContext for propagation as entries in a map // of unicode strings. -class TextMapWriter : public CarrierWriter { +// +// See the HTTPHeaders examples. +class TextMapWriter { public: + virtual ~TextMapWriter() = default; + // Set a key:value pair to the carrier. Multiple calls to Set() for the // same key leads to undefined behavior. // @@ -188,12 +133,58 @@ class TextMapWriter : public CarrierWriter { // HTTPHeadersReader is the Inject() carrier for the HttpHeaders builtin format. // With it, the caller can encode a SpanContext for propagation as entries in // http request headers. +// +// For example, Extract(): +// +// const Tracer& tracer = /* some tracer */ +// const HTTPHeadersReader& carrier_reader = /* some carrier */ +// auto span_context_maybe = tracer.Extract(carrier_reader); +// if (!span_context_maybe) { +// throw std::runtime_error(span_context_maybe.error().message()); +// } +// auto span = tracer.StartSpan("op", +// { ChildOf(span_context_maybe->get()) }); class HTTPHeadersReader : public TextMapReader {}; // HTTPHeadersWriter is the Inject() carrier for the TextMap builtin format. // With it, the caller can encode a SpanContext for propagation as entries in // http request headers +// +// For example, Inject(): +// +// const HTTPHeadersWriter& carrier_writer = /* some carrier */ +// auto was_successful = span.tracer().Inject(span, +// carrier_writer); +// if (!was_successful) { +// throw std::runtime_error(was_successful.error().message()); +// } class HTTPHeadersWriter : public TextMapWriter {}; + +// CustomCarrierReader is the Inject() carier for a custom format. With it, the +// caller can encode a SpanContext for propagation as entries in a custom +// protocol. +class CustomCarrierReader { + public: + virtual ~CustomCarrierReader() = default; + + // Extract is expected to specialize on the tracer implementation so as to + // most efficiently decode its context. + virtual Expected> Extract( + const Tracer& tracer) const = 0; +}; + +// CustomCarrierWriter is the Inject() carrier for a custom format. With it, +// the caller can encode a SpanContext for propagation as entries in a custom +// protocol. +class CustomCarrierWriter { + public: + virtual ~CustomCarrierWriter() = default; + + // Inject is expected to specialize on the tracer implementation so as to most + // efficiently encode its context. + virtual Expected Inject(const Tracer& tracer, + const SpanContext& sc) const = 0; +}; END_OPENTRACING_ABI_NAMESPACE } // namespace opentracing diff --git a/c++11/include/opentracing/tracer.h b/c++11/include/opentracing/tracer.h index 1fb1a15..37e49af 100644 --- a/c++11/include/opentracing/tracer.h +++ b/c++11/include/opentracing/tracer.h @@ -89,20 +89,25 @@ class Tracer { noexcept = 0; // Inject() takes the `sc` SpanContext instance and injects it for - // propagation within `carrier`. The actual type of `carrier` depends on - // the value of `format`. + // propagation within `carrier`. // - // OpenTracing defines a common set of `format` values (see BuiltinFormat), - // and each has an expected carrier type. + // OpenTracing defines a common set of `carrier` interfaces. // // Throws only if `writer` does. - virtual Expected Inject(const SpanContext& sc, CarrierFormat format, - const CarrierWriter& writer) const = 0; + virtual Expected Inject(const SpanContext& sc, + const TextMapWriter& writer) const = 0; - // Extract() returns a SpanContext instance given `format` and `carrier`. + virtual Expected Inject(const SpanContext& sc, + const HTTPHeadersWriter& writer) const = 0; + + virtual Expected Inject(const SpanContext& sc, + const CustomCarrierWriter& writer) const { + return writer.Inject(*this, sc); + } + + // Extract() returns a SpanContext instance given `carrier`. // - // OpenTracing defines a common set of `format` values (see BuiltinFormat), - // and each has an expected carrier type. + // OpenTracing defines a common set of `carrier` interfaces. // // Returns a `SpanContext` that is `non-null` on success or nullptr if // no span is found; otherwise an std::error_code from @@ -110,7 +115,15 @@ class Tracer { // // Throws only if `reader` does. virtual Expected> Extract( - CarrierFormat format, const CarrierReader& reader) const = 0; + const TextMapReader& reader) const = 0; + + virtual Expected> Extract( + const HTTPHeadersReader& reader) const = 0; + + virtual Expected> Extract( + const CustomCarrierReader& reader) const { + return reader.Extract(*this); + } // Close is called when a tracer is finished processing spans. It is not // required to be called and its effect is unspecified. For example, an diff --git a/c++11/src/noop.cpp b/c++11/src/noop.cpp index bc888ec..435e267 100644 --- a/c++11/src/noop.cpp +++ b/c++11/src/noop.cpp @@ -44,14 +44,22 @@ class NoopTracer : public Tracer, NoopSpan(shared_from_this())); } - Expected Inject(const SpanContext& /*sc*/, CarrierFormat /*format*/, - const CarrierWriter& /*writer*/) const override { + Expected Inject(const SpanContext& /*sc*/, + const TextMapWriter& /*writer*/) const override { return {}; } + Expected Inject(const SpanContext& /*sc*/, + const HTTPHeadersWriter& /*writer*/) const override { + return {}; + } + + Expected> Extract( + const TextMapReader& /*reader*/) const override { + return std::unique_ptr(nullptr); + } Expected> Extract( - CarrierFormat /*format*/, - const CarrierReader& /*reader*/) const override { + const HTTPHeadersReader& /*reader*/) const override { return std::unique_ptr(nullptr); } }; From 3bd4c3df9159fafd3e501fe9d196afccd53e4f9b Mon Sep 17 00:00:00 2001 From: Ryan Burn Date: Thu, 22 Jun 2017 18:50:12 -0400 Subject: [PATCH 41/42] Remove unsupported_format_error. --- c++11/include/opentracing/propagation.h | 5 ----- c++11/src/propagation.cpp | 6 ------ 2 files changed, 11 deletions(-) diff --git a/c++11/include/opentracing/propagation.h b/c++11/include/opentracing/propagation.h index ed06c00..83bd412 100644 --- a/c++11/include/opentracing/propagation.h +++ b/c++11/include/opentracing/propagation.h @@ -64,11 +64,6 @@ enum class SpanReferenceType { // https://ned14.github.io/boost.outcome/md_doc_md_03-tutorial_b.html const std::error_category& propagation_error_category(); -// `unsupported_format_error`` occurs when the `format` passed to -// Tracer::Inject() or Tracer::Extract() is not recognized by the Tracer -// implementation. -const std::error_code unsupported_format_error(0, propagation_error_category()); - // `invalid_span_context_error` errors occur when Tracer::Inject() is asked to // operate on a SpanContext which it is not prepared to handle (for // example, since it was created by a different tracer implementation). diff --git a/c++11/src/propagation.cpp b/c++11/src/propagation.cpp index a09dc95..daea776 100644 --- a/c++11/src/propagation.cpp +++ b/c++11/src/propagation.cpp @@ -11,9 +11,6 @@ class PropagationErrorCategory : public std::error_category { std::error_condition default_error_condition(int code) const noexcept override { - if (code == unsupported_format_error.value()) { - return std::make_error_condition(std::errc::not_supported); - } if (code == invalid_span_context_error.value()) { return std::make_error_condition(std::errc::not_supported); } @@ -27,9 +24,6 @@ class PropagationErrorCategory : public std::error_category { } std::string message(int code) const override { - if (code == unsupported_format_error.value()) { - return "opentracing: Unknown or unsupported Inject/Extract format"; - } if (code == invalid_span_context_error.value()) { return "opentracing: SpanContext type incompatible with tracer"; } From 681eff85398a7733778416ff154f573f783ed5bd Mon Sep 17 00:00:00 2001 From: Ryan Burn Date: Thu, 22 Jun 2017 19:01:49 -0400 Subject: [PATCH 42/42] Changed StringRef's API to more closely match std::string_view. --- c++11/include/opentracing/stringref.h | 46 +++-------------------- c++11/test/stringref_test.cpp | 53 --------------------------- 2 files changed, 6 insertions(+), 93 deletions(-) diff --git a/c++11/include/opentracing/stringref.h b/c++11/include/opentracing/stringref.h index 588f308..6c222e6 100644 --- a/c++11/include/opentracing/stringref.h +++ b/c++11/include/opentracing/stringref.h @@ -54,13 +54,8 @@ class StringRef { // Construct an empty StringRef StringRef() noexcept : data_(nullptr), length_(0) {} - // Explicitly create string reference from a const character array - template - StringRef(const char (&str)[N]) noexcept : data_(str), length_(N - 1) {} - - // Explicitly create string reference from const character pointer - explicit StringRef(const char* str) noexcept - : data_(str), length_(std::strlen(str)) {} + // create string reference from const character pointer + StringRef(const char* str) noexcept : data_(str), length_(std::strlen(str)) {} // Create constant string reference from pointer and length StringRef(const std::basic_string& str) noexcept @@ -69,47 +64,18 @@ class StringRef { // Create constant string reference from pointer and length StringRef(const char* str, size_t len) noexcept : data_(str), length_(len) {} - // Disallow construction from non-const array - template - StringRef(char (&str)[N]) = delete; - // Implicit conversion to std::string operator std::string() const { return {data_, length_}; } - // Reset the string reference given a const character array - template - void Reset(const char (&str)[N]) noexcept { - data_ = str; - length_ = N; - } - - // Reset this string ref to point at the supplied c-string - void Reset(const char* str) noexcept { - data_ = str; - length_ = std::strlen(str); - } - - // Reset the string reference given a std::string - void Reset(const std::basic_string& str) noexcept { - data_ = str.data(); - length_ = str.length(); - } - - // Reset this string ref to point at the supplied 'str' of 'length' bytes. - void Reset(const char* str, const size_t length) noexcept { - data_ = str; - length_ = length; - } - - // Disallow Reset from non-const array - template - void Reset(char (&str)[N]) = delete; - // Return address of the referenced string const char* data() const noexcept { return data_; } + // Returns true if `length_` == 0 + bool empty() const { return length_ == 0; } + // Return the length of the referenced string size_t length() const noexcept { return length_; } + size_t size() const noexcept { return length_; } // Returns a RandomAccessIterator to the first element. const char* begin() const noexcept { return data(); } diff --git a/c++11/test/stringref_test.cpp b/c++11/test/stringref_test.cpp index 41c9a65..f33d2ca 100644 --- a/c++11/test/stringref_test.cpp +++ b/c++11/test/stringref_test.cpp @@ -19,15 +19,6 @@ static void test_cstring() { assert(std::strlen(val) == ref.length()); } -static void test_cstring_array() { - const char val[] = "hello world"; - - StringRef ref(val); - - assert(val == ref.data()); - assert(std::strlen(val) == ref.length()); -} - static void test_std_string() { const std::string val = "hello world"; @@ -47,54 +38,10 @@ static void test_copy() { assert(val.length() == cpy.length()); } -static void test_reset() { - StringRef ref; - assert(0 == ref.data()); - assert(0 == ref.length()); - - ref.Reset("hello world"); - - assert("hello world" == ref); - assert(std::string("hello world") == std::string(ref.data())); - assert(std::strlen("hello world") == ref.length()); - - const char arr[] = "hello world 1"; - - ref.Reset(arr); - assert(arr == ref.data()); - assert(std::strlen(arr) == ref.length()); - assert(std::string(arr) == std::string(ref)); - assert(std::string(arr) == std::string(ref.data())); - - const char* p = "hello world 2"; - - ref.Reset(p); - assert(p == ref.data()); - assert(std::strlen(p) == ref.length()); - assert(std::string(p) == std::string(ref)); - assert(std::string(p) == std::string(ref.data())); - - const std::string s = "hello world 3"; - - ref.Reset(s); - assert(s.data() == ref.data()); - assert(s.length() == ref.length()); - assert(std::string(s.c_str()) == std::string(ref)); - assert(std::string(s.c_str()) == std::string(ref.data())); - - ref.Reset(p, std::strlen(p)); - assert(p == ref.data()); - assert(std::strlen(p) == ref.length()); - assert(p == ref); - assert(std::string(p) == std::string(ref.data())); -} - int main() { test_empty(); test_cstring(); - test_cstring_array(); test_std_string(); test_copy(); - test_reset(); return 0; }