From 52128785560b520ecf99e5c14d894cd137b03d66 Mon Sep 17 00:00:00 2001 From: David Goffredo Date: Fri, 30 Sep 2022 10:59:50 -0400 Subject: [PATCH 1/8] DRY numeric_limits, and str -> string --- src/datadog/msgpack.h | 28 ++++++++++++++-------------- src/datadog/span_data.cpp | 38 +++++++++++++++++++------------------- 2 files changed, 33 insertions(+), 33 deletions(-) diff --git a/src/datadog/msgpack.h b/src/datadog/msgpack.h index 06fe03af..4d1a5cbf 100644 --- a/src/datadog/msgpack.h +++ b/src/datadog/msgpack.h @@ -30,10 +30,10 @@ void pack_integer(std::string& buffer, Integer value); void pack_double(std::string& buffer, double value); -void pack_str(std::string& buffer, const char* cstr); +void pack_string(std::string& buffer, const char* cstr); template -void pack_str(std::string& buffer, const Range& range); +void pack_string(std::string& buffer, const Range& range); void pack_array(std::string& buffer, size_t size); @@ -139,17 +139,17 @@ inline void pack_double(std::string& buffer, double value) { push_number_big_endian(buffer, memory.as_integer); } -inline void pack_str(std::string& buffer, const char* cstr) { - return pack_str(buffer, std::string_view(cstr)); +inline void pack_string(std::string& buffer, const char* cstr) { + return pack_string(buffer, std::string_view(cstr)); } template -void pack_str(std::string& buffer, const Range& range) { +void pack_string(std::string& buffer, const Range& range) { auto size = static_cast(std::distance(std::begin(range), std::end(range))); - if (size > std::numeric_limits::max()) { - throw std::out_of_range(make_overflow_message( - "string", size, std::numeric_limits::max())); + const auto max = std::numeric_limits::max(); + if (size > max) { + throw std::out_of_range(make_overflow_message("string", size, max)); } buffer.push_back(static_cast(types::STR32)); push_number_big_endian(buffer, static_cast(size)); @@ -157,18 +157,18 @@ void pack_str(std::string& buffer, const Range& range) { } inline void pack_array(std::string& buffer, size_t size) { - if (size > std::numeric_limits::max()) { - throw std::out_of_range(make_overflow_message( - "array", size, std::numeric_limits::max())); + const auto max = std::numeric_limits::max(); + if (size > max) { + throw std::out_of_range(make_overflow_message("array", size, max)); } buffer.push_back(static_cast(types::ARRAY32)); push_number_big_endian(buffer, static_cast(size)); } inline void pack_map(std::string& buffer, size_t size) { - if (size > std::numeric_limits::max()) { - throw std::out_of_range(make_overflow_message( - "map", size, std::numeric_limits::max())); + const auto max = std::numeric_limits::max(); + if (size > max) { + throw std::out_of_range(make_overflow_message("map", size, max)); } buffer.push_back(static_cast(types::MAP32)); push_number_big_endian(buffer, static_cast(size)); diff --git a/src/datadog/span_data.cpp b/src/datadog/span_data.cpp index ee962c0d..94179c2e 100644 --- a/src/datadog/span_data.cpp +++ b/src/datadog/span_data.cpp @@ -68,55 +68,55 @@ Expected msgpack_encode(std::string& destination, const int num_fields = 12; msgpack::pack_map(destination, num_fields); - msgpack::pack_str(destination, "service"); - msgpack::pack_str(destination, span.service); + msgpack::pack_string(destination, "service"); + msgpack::pack_string(destination, span.service); - msgpack::pack_str(destination, "name"); - msgpack::pack_str(destination, span.name); + msgpack::pack_string(destination, "name"); + msgpack::pack_string(destination, span.name); - msgpack::pack_str(destination, "resource"); - msgpack::pack_str(destination, span.resource); + msgpack::pack_string(destination, "resource"); + msgpack::pack_string(destination, span.resource); - msgpack::pack_str(destination, "trace_id"); + msgpack::pack_string(destination, "trace_id"); msgpack::pack_integer(destination, span.trace_id); - msgpack::pack_str(destination, "span_id"); + msgpack::pack_string(destination, "span_id"); msgpack::pack_integer(destination, span.span_id); - msgpack::pack_str(destination, "parent_id"); + msgpack::pack_string(destination, "parent_id"); msgpack::pack_integer(destination, span.parent_id); - msgpack::pack_str(destination, "start"); + msgpack::pack_string(destination, "start"); msgpack::pack_integer(destination, std::chrono::duration_cast( span.start.wall.time_since_epoch()) .count()); - msgpack::pack_str(destination, "duration"); + msgpack::pack_string(destination, "duration"); msgpack::pack_integer( destination, std::chrono::duration_cast(span.duration) .count()); - msgpack::pack_str(destination, "error"); + msgpack::pack_string(destination, "error"); msgpack::pack_integer(destination, std::int32_t(span.error)); - msgpack::pack_str(destination, "meta"); + msgpack::pack_string(destination, "meta"); msgpack::pack_map(destination, span.tags.size()); for (const auto& [key, value] : span.tags) { - msgpack::pack_str(destination, key); - msgpack::pack_str(destination, value); + msgpack::pack_string(destination, key); + msgpack::pack_string(destination, value); } - msgpack::pack_str(destination, "metrics"); + msgpack::pack_string(destination, "metrics"); msgpack::pack_map(destination, span.numeric_tags.size()); for (const auto& [key, value] : span.numeric_tags) { - msgpack::pack_str(destination, key); + msgpack::pack_string(destination, key); msgpack::pack_double(destination, value); } - msgpack::pack_str(destination, "type"); - msgpack::pack_str(destination, span.service_type); + msgpack::pack_string(destination, "type"); + msgpack::pack_string(destination, span.service_type); // Be sure to update `num_fields` when adding fields. return std::nullopt; From cea53c7fc6b9849210ea95bd2acfe3af9e337098 Mon Sep 17 00:00:00 2001 From: David Goffredo Date: Fri, 30 Sep 2022 11:41:39 -0400 Subject: [PATCH 2/8] automatically pack unordered_map --- src/datadog/msgpack.cpp | 124 ++++++++++++++++++++++++++++ src/datadog/msgpack.h | 168 ++++++-------------------------------- src/datadog/span_data.cpp | 21 +++-- 3 files changed, 160 insertions(+), 153 deletions(-) diff --git a/src/datadog/msgpack.cpp b/src/datadog/msgpack.cpp index 661f76cf..d8e9f354 100644 --- a/src/datadog/msgpack.cpp +++ b/src/datadog/msgpack.cpp @@ -1 +1,125 @@ #include "msgpack.h" + +#include +#include +#include +#include + +namespace datadog { +namespace tracing { +namespace msgpack { +namespace { +// MessagePack values are prefixed by a byte naming their type. +namespace types { +constexpr auto ARRAY32 = std::byte(0xDD); +constexpr auto DOUBLE = std::byte(0xCB); +constexpr auto INT64 = std::byte(0xD3); +constexpr auto MAP32 = std::byte(0xDF); +constexpr auto STR32 = std::byte(0xDB); +constexpr auto UINT64 = std::byte(0xCF); +} // namespace types +} // namespace + +std::string make_overflow_message(std::string_view type, std::size_t actual, + std::size_t max) { + std::string message; + message += "Cannot msgpack encode "; + message += type; + message += " of size "; + message += std::to_string(actual); + message += ", which exceeds the protocol maximum of "; + message += std::to_string(max); + message += '.'; + return message; +} + +template +void push_number_big_endian(std::string& buffer, Integer integer) { + // Assume two's complement. + const std::make_unsigned_t value = integer; + + // The loop below is more likely to unroll if we don't call any functions + // within it. + char buf[sizeof value]; + + // The most significant byte of `value` goes to the front of `buf`, and the + // least significant byte of `value` goes + // to the back of `buf`, and so on in between. + // On a big endian architecture, this is just a complicated way to copy + // `value`. On a little endian architecture, which is much more common, this + // effectively copies the bytes of `value` backwards. + const int size = sizeof value; + for (int i = 0; i < size; ++i) { + const char byte = (value >> (8 * ((size - 1) - i))) & 0xFF; + buf[i] = byte; + } + + buffer.append(buf, sizeof buf); +} + +void pack_negative(std::string& buffer, std::int64_t value) { + buffer.push_back(static_cast(types::INT64)); + push_number_big_endian(buffer, static_cast(value)); +} + +void pack_nonnegative(std::string& buffer, std::uint64_t value) { + buffer.push_back(static_cast(types::UINT64)); + push_number_big_endian(buffer, static_cast(value)); +} + +void pack_double(std::string& buffer, double value) { + buffer.push_back(static_cast(types::DOUBLE)); + + // The following is lifted from the "msgpack-c" project. + // See "pack_double" in + // + + union { + double as_double; + uint64_t as_integer; + } memory; + memory.as_double = value; + +#if defined(TARGET_OS_IPHONE) + // ok +#elif defined(__arm__) && !(__ARM_EABI__) // arm-oabi + // https://github.com/msgpack/msgpack-perl/pull/1 + memory.as_integer = + (memory.as_integer & 0xFFFFFFFFUL) << 32UL | (memory.as_integer >> 32UL); +#endif + + push_number_big_endian(buffer, memory.as_integer); +} + +void pack_string(std::string& buffer, std::string_view value) { + const auto size = value.size(); + const auto max = std::numeric_limits::max(); + if (size > max) { + throw std::out_of_range(make_overflow_message("string", size, max)); + } + buffer.push_back(static_cast(types::STR32)); + push_number_big_endian(buffer, static_cast(size)); + buffer.append(value.begin(), value.end()); +} + +void pack_array(std::string& buffer, size_t size) { + const auto max = std::numeric_limits::max(); + if (size > max) { + throw std::out_of_range(make_overflow_message("array", size, max)); + } + buffer.push_back(static_cast(types::ARRAY32)); + push_number_big_endian(buffer, static_cast(size)); +} + +void pack_map(std::string& buffer, size_t size) { + const auto max = std::numeric_limits::max(); + if (size > max) { + throw std::out_of_range(make_overflow_message("map", size, max)); + } + buffer.push_back(static_cast(types::MAP32)); + push_number_big_endian(buffer, static_cast(size)); +} + +} // namespace msgpack +} // namespace tracing +} // namespace datadog diff --git a/src/datadog/msgpack.h b/src/datadog/msgpack.h index 4d1a5cbf..1b94ae3b 100644 --- a/src/datadog/msgpack.h +++ b/src/datadog/msgpack.h @@ -2,109 +2,25 @@ #include #include -#include -#include #include #include -#include namespace datadog { namespace tracing { namespace msgpack { -// First, declare all functions, so that I don't have to topologically sort -// their inline definitions. - -template -void push_number_big_endian(std::string& buffer, Integer integer); - -template -void push(std::string& buffer, const Range& range); - void pack_negative(std::string& buffer, std::int64_t value); - void pack_nonnegative(std::string& buffer, std::uint64_t value); -template -void pack_integer(std::string& buffer, Integer value); - void pack_double(std::string& buffer, double value); +void pack_string(std::string& buffer, std::string_view value); -void pack_string(std::string& buffer, const char* cstr); - -template -void pack_string(std::string& buffer, const Range& range); - -void pack_array(std::string& buffer, size_t size); - -void pack_map(std::string& buffer, size_t size); - -std::string make_overflow_message(std::string_view type, std::size_t actual, - std::size_t max); - -// MessagePack values are prefixed by a byte naming their type. -namespace types { -constexpr auto DOUBLE = std::byte(0xCB); -constexpr auto UINT64 = std::byte(0xCF); -constexpr auto INT64 = std::byte(0xD3); -constexpr auto STR32 = std::byte(0xDB); -constexpr auto ARRAY32 = std::byte(0xDD); -constexpr auto MAP32 = std::byte(0xDF); -} // namespace types - -// Here are the inline definitions of all of the functions declared above. - -inline std::string make_overflow_message(std::string_view type, - std::size_t actual, std::size_t max) { - std::string message; - message += "Cannot msgpack encode "; - message += type; - message += " of size "; - message += std::to_string(actual); - message += ", which exceeds the protocol maximum of "; - message += std::to_string(max); - message += '.'; - return message; -} - -template -void push_number_big_endian(std::string& buffer, Integer integer) { - // Assume two's complement. - const std::make_unsigned_t value = integer; - - // The loop below is more likely to unroll if we don't call any functions - // within it. - char buf[sizeof value]; - - // The most significant byte of `value` goes to the front of `buf`, and the - // least significant byte of `value` goes - // to the back of `buf`, and so on in between. - // On a big endian architecture, this is just a complicated way to copy - // `value`. On a little endian architecture, which is much more common, this - // effectively copies the bytes of `value` backwards. - const int size = sizeof value; - for (int i = 0; i < size; ++i) { - const char byte = (value >> (8 * ((size - 1) - i))) & 0xFF; - buf[i] = byte; - } - - buffer.append(buf, sizeof buf); -} - -template -void push(std::string& buffer, const Range& range) { - buffer.insert(buffer.end(), std::begin(range), std::end(range)); -} +void pack_array(std::string& buffer, std::size_t size); +void pack_map(std::string& buffer, std::size_t size); -inline void pack_negative(std::string& buffer, std::int64_t value) { - buffer.push_back(static_cast(types::INT64)); - push_number_big_endian(buffer, static_cast(value)); -} - -inline void pack_nonnegative(std::string& buffer, std::uint64_t value) { - buffer.push_back(static_cast(types::UINT64)); - push_number_big_endian(buffer, static_cast(value)); -} +template +void pack_map_suffix(std::string& buffer, Entry&& entry, Entries&&... entries); +void pack_map_suffix(std::string& buffer); template void pack_integer(std::string& buffer, Integer value) { @@ -115,65 +31,33 @@ void pack_integer(std::string& buffer, Integer value) { } } -inline void pack_double(std::string& buffer, double value) { - buffer.push_back(static_cast(types::DOUBLE)); - - // The following is lifted from the "msgpack-c" project. - // See "pack_double" in - // - - union { - double as_double; - uint64_t as_integer; - } memory; - memory.as_double = value; - -#if defined(TARGET_OS_IPHONE) - // ok -#elif defined(__arm__) && !(__ARM_EABI__) // arm-oabi - // https://github.com/msgpack/msgpack-perl/pull/1 - memory.as_integer = - (memory.as_integer & 0xFFFFFFFFUL) << 32UL | (memory.as_integer >> 32UL); -#endif - - push_number_big_endian(buffer, memory.as_integer); -} - -inline void pack_string(std::string& buffer, const char* cstr) { - return pack_string(buffer, std::string_view(cstr)); -} - -template -void pack_string(std::string& buffer, const Range& range) { - auto size = - static_cast(std::distance(std::begin(range), std::end(range))); - const auto max = std::numeric_limits::max(); - if (size > max) { - throw std::out_of_range(make_overflow_message("string", size, max)); +template +void pack_map(std::string& buffer, const PairIterable& pairs, + PackValue&& pack_value) { + pack_map(buffer, std::size(pairs)); + for (const auto& [key, value] : pairs) { + pack_string(buffer, key); + pack_value(buffer, value); } - buffer.push_back(static_cast(types::STR32)); - push_number_big_endian(buffer, static_cast(size)); - push(buffer, range); } -inline void pack_array(std::string& buffer, size_t size) { - const auto max = std::numeric_limits::max(); - if (size > max) { - throw std::out_of_range(make_overflow_message("array", size, max)); - } - buffer.push_back(static_cast(types::ARRAY32)); - push_number_big_endian(buffer, static_cast(size)); +template +void pack_map(std::string& buffer, Entry&& entry, Entries&&... entries) { + pack_map(buffer, 1 + sizeof...(entries)); + pack_map_suffix(buffer, std::forward(entry), + std::forward(entries)...); } -inline void pack_map(std::string& buffer, size_t size) { - const auto max = std::numeric_limits::max(); - if (size > max) { - throw std::out_of_range(make_overflow_message("map", size, max)); - } - buffer.push_back(static_cast(types::MAP32)); - push_number_big_endian(buffer, static_cast(size)); +template +void pack_map_suffix(std::string& buffer, Entry&& entry, Entries&&... entries) { + auto&& [key, pack_value] = entry; + pack_string(buffer, key); + pack_value(buffer); + pack_map_suffix(buffer, std::forward(entries)...); } +inline void pack_map_suffix(std::string&) {} + } // namespace msgpack } // namespace tracing } // namespace datadog diff --git a/src/datadog/span_data.cpp b/src/datadog/span_data.cpp index 94179c2e..b187e24c 100644 --- a/src/datadog/span_data.cpp +++ b/src/datadog/span_data.cpp @@ -1,5 +1,6 @@ #include "span_data.h" +#include #include #include @@ -65,7 +66,7 @@ void SpanData::apply_config(const SpanDefaults& defaults, Expected msgpack_encode(std::string& destination, const SpanData& span) try { // Be sure to update `num_fields` when adding fields. - const int num_fields = 12; + const std::size_t num_fields = 12; msgpack::pack_map(destination, num_fields); msgpack::pack_string(destination, "service"); @@ -102,18 +103,16 @@ Expected msgpack_encode(std::string& destination, msgpack::pack_integer(destination, std::int32_t(span.error)); msgpack::pack_string(destination, "meta"); - msgpack::pack_map(destination, span.tags.size()); - for (const auto& [key, value] : span.tags) { - msgpack::pack_string(destination, key); - msgpack::pack_string(destination, value); - } + msgpack::pack_map(destination, span.tags, + [](std::string& destination, const auto& value) { + msgpack::pack_string(destination, value); + }); msgpack::pack_string(destination, "metrics"); - msgpack::pack_map(destination, span.numeric_tags.size()); - for (const auto& [key, value] : span.numeric_tags) { - msgpack::pack_string(destination, key); - msgpack::pack_double(destination, value); - } + msgpack::pack_map(destination, span.numeric_tags, + [](std::string& destination, const auto& value) { + msgpack::pack_double(destination, value); + }); msgpack::pack_string(destination, "type"); msgpack::pack_string(destination, span.service_type); From 607dc6177ee44b3c30d1f3e52a68f22287c221d5 Mon Sep 17 00:00:00 2001 From: David Goffredo Date: Fri, 30 Sep 2022 13:12:11 -0400 Subject: [PATCH 3/8] pack a struct without having to name how many fields --- src/datadog/msgpack.cpp | 3 +- src/datadog/msgpack.h | 36 +++++++++----- src/datadog/span_data.cpp | 101 +++++++++++++++++++------------------- 3 files changed, 76 insertions(+), 64 deletions(-) diff --git a/src/datadog/msgpack.cpp b/src/datadog/msgpack.cpp index d8e9f354..10054900 100644 --- a/src/datadog/msgpack.cpp +++ b/src/datadog/msgpack.cpp @@ -18,7 +18,6 @@ constexpr auto MAP32 = std::byte(0xDF); constexpr auto STR32 = std::byte(0xDB); constexpr auto UINT64 = std::byte(0xCF); } // namespace types -} // namespace std::string make_overflow_message(std::string_view type, std::size_t actual, std::size_t max) { @@ -57,6 +56,8 @@ void push_number_big_endian(std::string& buffer, Integer integer) { buffer.append(buf, sizeof buf); } +} // namespace + void pack_negative(std::string& buffer, std::int64_t value) { buffer.push_back(static_cast(types::INT64)); push_number_big_endian(buffer, static_cast(value)); diff --git a/src/datadog/msgpack.h b/src/datadog/msgpack.h index 1b94ae3b..ca33d548 100644 --- a/src/datadog/msgpack.h +++ b/src/datadog/msgpack.h @@ -4,6 +4,7 @@ #include #include #include +#include namespace datadog { namespace tracing { @@ -17,9 +18,16 @@ void pack_string(std::string& buffer, std::string_view value); void pack_array(std::string& buffer, std::size_t size); void pack_map(std::string& buffer, std::size_t size); +template +void pack_map(std::string& buffer, const PairIterable& pairs, + PackValue&& pack_value); +template +void pack_map(std::string& buffer, Key&& key, PackValue&& pack_value, + Rest&&... rest); -template -void pack_map_suffix(std::string& buffer, Entry&& entry, Entries&&... entries); +template +void pack_map_suffix(std::string& buffer, Key&& key, PackValue&& pack_value, + Rest&&... rest); void pack_map_suffix(std::string& buffer); template @@ -41,22 +49,26 @@ void pack_map(std::string& buffer, const PairIterable& pairs, } } -template -void pack_map(std::string& buffer, Entry&& entry, Entries&&... entries) { - pack_map(buffer, 1 + sizeof...(entries)); - pack_map_suffix(buffer, std::forward(entry), - std::forward(entries)...); +template +void pack_map(std::string& buffer, Key&& key, PackValue&& pack_value, + Rest&&... rest) { + pack_map(buffer, 1 + sizeof...(rest)); + pack_map_suffix(buffer, std::forward(key), + std::forward(pack_value), + std::forward(rest)...); } -template -void pack_map_suffix(std::string& buffer, Entry&& entry, Entries&&... entries) { - auto&& [key, pack_value] = entry; +template +void pack_map_suffix(std::string& buffer, Key&& key, PackValue&& pack_value, + Rest&&... rest) { pack_string(buffer, key); pack_value(buffer); - pack_map_suffix(buffer, std::forward(entries)...); + pack_map_suffix(buffer, std::forward(rest)...); } -inline void pack_map_suffix(std::string&) {} +inline void pack_map_suffix(std::string&) { + // base case does nothing +} } // namespace msgpack } // namespace tracing diff --git a/src/datadog/span_data.cpp b/src/datadog/span_data.cpp index b187e24c..8056be0f 100644 --- a/src/datadog/span_data.cpp +++ b/src/datadog/span_data.cpp @@ -65,59 +65,58 @@ void SpanData::apply_config(const SpanDefaults& defaults, Expected msgpack_encode(std::string& destination, const SpanData& span) try { - // Be sure to update `num_fields` when adding fields. - const std::size_t num_fields = 12; - msgpack::pack_map(destination, num_fields); - - msgpack::pack_string(destination, "service"); - msgpack::pack_string(destination, span.service); - - msgpack::pack_string(destination, "name"); - msgpack::pack_string(destination, span.name); - - msgpack::pack_string(destination, "resource"); - msgpack::pack_string(destination, span.resource); - - msgpack::pack_string(destination, "trace_id"); - msgpack::pack_integer(destination, span.trace_id); - - msgpack::pack_string(destination, "span_id"); - msgpack::pack_integer(destination, span.span_id); - - msgpack::pack_string(destination, "parent_id"); - msgpack::pack_integer(destination, span.parent_id); - - msgpack::pack_string(destination, "start"); - msgpack::pack_integer(destination, - std::chrono::duration_cast( - span.start.wall.time_since_epoch()) - .count()); - - msgpack::pack_string(destination, "duration"); - msgpack::pack_integer( + // clang-format off + msgpack::pack_map( destination, - std::chrono::duration_cast(span.duration) - .count()); - - msgpack::pack_string(destination, "error"); - msgpack::pack_integer(destination, std::int32_t(span.error)); - - msgpack::pack_string(destination, "meta"); - msgpack::pack_map(destination, span.tags, - [](std::string& destination, const auto& value) { - msgpack::pack_string(destination, value); - }); - - msgpack::pack_string(destination, "metrics"); - msgpack::pack_map(destination, span.numeric_tags, - [](std::string& destination, const auto& value) { - msgpack::pack_double(destination, value); - }); - - msgpack::pack_string(destination, "type"); - msgpack::pack_string(destination, span.service_type); + "service", [&](auto& destination) { + msgpack::pack_string(destination, span.service); + }, + "name", [&](auto& destination) { + msgpack::pack_string(destination, span.name); + }, + "resource", [&](auto& destination) { + msgpack::pack_string(destination, span.resource); + }, + "trace_id", [&](auto& destination) { + msgpack::pack_integer(destination, span.trace_id); + }, + "span_id", [&](auto& destination) { + msgpack::pack_integer(destination, span.span_id); + }, + "parent_id", [&](auto& destination) { + msgpack::pack_integer(destination, span.parent_id); + }, + "start", [&](auto& destination) { + msgpack::pack_integer( + destination, std::chrono::duration_cast( + span.start.wall.time_since_epoch()) + .count()); + }, + "duration", [&](auto& destination) { + msgpack::pack_integer( + destination, + std::chrono::duration_cast(span.duration) + .count()); + }, + "error", [&](auto& destination) { + msgpack::pack_integer(destination, std::int32_t(span.error)); + }, + "meta", [&](auto& destination) { + msgpack::pack_map(destination, span.tags, + [](std::string& destination, const auto& value) { + msgpack::pack_string(destination, value); + }); + }, "metrics", + [&](auto& destination) { + msgpack::pack_map(destination, span.numeric_tags, + [](std::string& destination, const auto& value) { + msgpack::pack_double(destination, value); + }); + }, "type", [&](auto& destination) { + msgpack::pack_string(destination, span.service_type); + }); + // clang-format on - // Be sure to update `num_fields` when adding fields. return std::nullopt; } catch (const std::exception& error) { return Error{Error::MESSAGEPACK_ENCODE_FAILURE, error.what()}; From 734f441d7a47ee977990b93d7f94e2199764aee4 Mon Sep 17 00:00:00 2001 From: David Goffredo Date: Fri, 30 Sep 2022 13:24:37 -0400 Subject: [PATCH 4/8] msgpack encode vectors --- src/datadog/datadog_agent.cpp | 30 +++++++++--------------------- src/datadog/msgpack.h | 20 ++++++++++++++++++++ 2 files changed, 29 insertions(+), 21 deletions(-) diff --git a/src/datadog/datadog_agent.cpp b/src/datadog/datadog_agent.cpp index 1d68a241..d553543c 100644 --- a/src/datadog/datadog_agent.cpp +++ b/src/datadog/datadog_agent.cpp @@ -32,17 +32,11 @@ HTTPClient::URL traces_endpoint(const HTTPClient::URL& agent_url) { Expected msgpack_encode( std::string& destination, const std::vector>& spans) try { - msgpack::pack_array(destination, spans.size()); - - for (const auto& span_ptr : spans) { - assert(span_ptr); - auto result = msgpack_encode(destination, *span_ptr); - if (auto* error = result.if_error()) { - return std::move(*error); - } - } - - return std::nullopt; + return msgpack::pack_array(destination, spans, + [](auto& destination, const auto& span_ptr) { + assert(span_ptr); + return msgpack_encode(destination, *span_ptr); + }); } catch (const std::exception& error) { return Error{Error::MESSAGEPACK_ENCODE_FAILURE, error.what()}; } @@ -50,16 +44,10 @@ Expected msgpack_encode( Expected msgpack_encode( std::string& destination, const std::vector& trace_chunks) try { - msgpack::pack_array(destination, trace_chunks.size()); - - for (const auto& chunk : trace_chunks) { - auto result = msgpack_encode(destination, chunk.spans); - if (auto* error = result.if_error()) { - return std::move(*error); - } - } - - return std::nullopt; + return msgpack::pack_array(destination, trace_chunks, + [](auto& destination, const auto& chunk) { + return msgpack_encode(destination, chunk.spans); + }); } catch (const std::exception& error) { return Error{Error::MESSAGEPACK_ENCODE_FAILURE, error.what()}; } diff --git a/src/datadog/msgpack.h b/src/datadog/msgpack.h index ca33d548..525c1923 100644 --- a/src/datadog/msgpack.h +++ b/src/datadog/msgpack.h @@ -6,6 +6,8 @@ #include #include +#include "expected.h" + namespace datadog { namespace tracing { namespace msgpack { @@ -17,6 +19,10 @@ void pack_double(std::string& buffer, double value); void pack_string(std::string& buffer, std::string_view value); void pack_array(std::string& buffer, std::size_t size); +template +Expected pack_array(std::string& buffer, Iterable&& values, + PackValue&& pack_value); + void pack_map(std::string& buffer, std::size_t size); template void pack_map(std::string& buffer, const PairIterable& pairs, @@ -39,6 +45,20 @@ void pack_integer(std::string& buffer, Integer value) { } } +template +Expected pack_array(std::string& buffer, Iterable&& values, + PackValue&& pack_value) { + Expected result; + pack_array(buffer, std::size(values)); + for (const auto& value : values) { + result = pack_value(buffer, value); + if (!result) { + break; + } + } + return result; +} + template void pack_map(std::string& buffer, const PairIterable& pairs, PackValue&& pack_value) { From 21ceb47037776ee3629036a3ddfbcec80424a8c5 Mon Sep 17 00:00:00 2001 From: David Goffredo Date: Fri, 30 Sep 2022 16:21:38 -0400 Subject: [PATCH 5/8] document the new MessagePack encoding functions --- src/datadog/msgpack.h | 34 ++++++++++++++++++++++++++++++++++ 1 file changed, 34 insertions(+) diff --git a/src/datadog/msgpack.h b/src/datadog/msgpack.h index 525c1923..7db117b2 100644 --- a/src/datadog/msgpack.h +++ b/src/datadog/msgpack.h @@ -1,5 +1,16 @@ #pragma once +// This component provides encoding routines for [MessagePack][1]. +// +// Each function is in `namespace msgpack` and appends a specified value to a +// `std::string`. For example, `msgpack::pack_integer(destination, -42)` +// MessagePack encodes the number `-42` and appends the result to `destination`. +// +// Only encoding is provided, and only for the types required by `SpanData` and +// `DatadogAgent`. +// +// [1]: https://msgpack.org/index.html + #include #include #include @@ -19,14 +30,37 @@ void pack_double(std::string& buffer, double value); void pack_string(std::string& buffer, std::string_view value); void pack_array(std::string& buffer, std::size_t size); + +// Append to the specified `buffer` a MessagePack encoded array having the +// specified `values`, where for each element of `values` the specified +// `pack_value` function appends the value. `pack_value` is invoked with two +// arguments: the first is a reference to `buffer`, and the second is a +// reference to the current value. `pack_value` returns an `Expected`. +// If the return value is an error, then iteration is halted and the error is +// returned. Otherwise the non-error value is returned. template Expected pack_array(std::string& buffer, Iterable&& values, PackValue&& pack_value); void pack_map(std::string& buffer, std::size_t size); + +// Append to the specified `buffer` a MessagePack encoded map consisting of the +// specified `pairs`, where the first element of each pair is the name of the +// map element, and the second element of each pair is some value that is +// MessagePack encoded by the specified `pack_value` function. `pack_value` is +// invoked with two arguments: the first is a reference to `buffer`, and the +// second is a reference to the current value. template void pack_map(std::string& buffer, const PairIterable& pairs, PackValue&& pack_value); + +// Append to the specified `buffer` a MessagePack encoded map consisting of the +// specified key value pairs. After the `buffer` argument, `pack_map` accepts +// an even number of arguments. First in each pair of arguments is `key`, the +// key name of the corresponding map item. Second in each pair of arguments is +// `pack_value`, a function that encodes the corresponding value. `pack_value` +// is invoked with two arguments: the first is a reference to `buffer`, and the +// second is a reference to the current value. template void pack_map(std::string& buffer, Key&& key, PackValue&& pack_value, Rest&&... rest); From 6f99e8e873d5b44d8cdca566e678d23bd72eda22 Mon Sep 17 00:00:00 2001 From: David Goffredo Date: Fri, 30 Sep 2022 16:30:36 -0400 Subject: [PATCH 6/8] remove pack_integer template --- src/datadog/msgpack.cpp | 4 ++-- src/datadog/msgpack.h | 17 ++++++----------- 2 files changed, 8 insertions(+), 13 deletions(-) diff --git a/src/datadog/msgpack.cpp b/src/datadog/msgpack.cpp index 10054900..83ba7810 100644 --- a/src/datadog/msgpack.cpp +++ b/src/datadog/msgpack.cpp @@ -58,12 +58,12 @@ void push_number_big_endian(std::string& buffer, Integer integer) { } // namespace -void pack_negative(std::string& buffer, std::int64_t value) { +void pack_integer(std::string& buffer, std::int64_t value) { buffer.push_back(static_cast(types::INT64)); push_number_big_endian(buffer, static_cast(value)); } -void pack_nonnegative(std::string& buffer, std::uint64_t value) { +void pack_integer(std::string& buffer, std::uint64_t value) { buffer.push_back(static_cast(types::UINT64)); push_number_big_endian(buffer, static_cast(value)); } diff --git a/src/datadog/msgpack.h b/src/datadog/msgpack.h index 7db117b2..6c632747 100644 --- a/src/datadog/msgpack.h +++ b/src/datadog/msgpack.h @@ -23,8 +23,12 @@ namespace datadog { namespace tracing { namespace msgpack { -void pack_negative(std::string& buffer, std::int64_t value); -void pack_nonnegative(std::string& buffer, std::uint64_t value); +void pack_integer(std::string& buffer, std::int64_t value); +void pack_integer(std::string& buffer, std::uint64_t value); + +inline void pack_integer(std::string& buffer, std::int32_t value) { + pack_integer(buffer, std::int64_t(value)); +} void pack_double(std::string& buffer, double value); void pack_string(std::string& buffer, std::string_view value); @@ -70,15 +74,6 @@ void pack_map_suffix(std::string& buffer, Key&& key, PackValue&& pack_value, Rest&&... rest); void pack_map_suffix(std::string& buffer); -template -void pack_integer(std::string& buffer, Integer value) { - if (value < 0) { - return pack_negative(buffer, value); - } else { - return pack_nonnegative(buffer, value); - } -} - template Expected pack_array(std::string& buffer, Iterable&& values, PackValue&& pack_value) { From 4bd47cbd8f6223e14ad1dfdf8005c121d2c032b5 Mon Sep 17 00:00:00 2001 From: David Goffredo Date: Fri, 30 Sep 2022 16:54:51 -0400 Subject: [PATCH 7/8] remove exceptions from msgpack --- src/datadog/msgpack.cpp | 21 +++++--- src/datadog/msgpack.h | 111 +++++++++++++++++++++++++------------- src/datadog/span_data.cpp | 21 +++++--- 3 files changed, 102 insertions(+), 51 deletions(-) diff --git a/src/datadog/msgpack.cpp b/src/datadog/msgpack.cpp index 83ba7810..6ea5fc7a 100644 --- a/src/datadog/msgpack.cpp +++ b/src/datadog/msgpack.cpp @@ -2,9 +2,10 @@ #include #include -#include #include +#include "error.h" + namespace datadog { namespace tracing { namespace msgpack { @@ -92,33 +93,39 @@ void pack_double(std::string& buffer, double value) { push_number_big_endian(buffer, memory.as_integer); } -void pack_string(std::string& buffer, std::string_view value) { +Expected pack_string(std::string& buffer, std::string_view value) { const auto size = value.size(); const auto max = std::numeric_limits::max(); if (size > max) { - throw std::out_of_range(make_overflow_message("string", size, max)); + return Error{Error::MESSAGEPACK_ENCODE_FAILURE, + make_overflow_message("string", size, max)}; } buffer.push_back(static_cast(types::STR32)); push_number_big_endian(buffer, static_cast(size)); buffer.append(value.begin(), value.end()); + return {}; } -void pack_array(std::string& buffer, size_t size) { +Expected pack_array(std::string& buffer, size_t size) { const auto max = std::numeric_limits::max(); if (size > max) { - throw std::out_of_range(make_overflow_message("array", size, max)); + return Error{Error::MESSAGEPACK_ENCODE_FAILURE, + make_overflow_message("array", size, max)}; } buffer.push_back(static_cast(types::ARRAY32)); push_number_big_endian(buffer, static_cast(size)); + return {}; } -void pack_map(std::string& buffer, size_t size) { +Expected pack_map(std::string& buffer, size_t size) { const auto max = std::numeric_limits::max(); if (size > max) { - throw std::out_of_range(make_overflow_message("map", size, max)); + return Error{Error::MESSAGEPACK_ENCODE_FAILURE, + make_overflow_message("map", size, max)}; } buffer.push_back(static_cast(types::MAP32)); push_number_big_endian(buffer, static_cast(size)); + return {}; } } // namespace msgpack diff --git a/src/datadog/msgpack.h b/src/datadog/msgpack.h index 6c632747..212350cf 100644 --- a/src/datadog/msgpack.h +++ b/src/datadog/msgpack.h @@ -25,38 +25,40 @@ namespace msgpack { void pack_integer(std::string& buffer, std::int64_t value); void pack_integer(std::string& buffer, std::uint64_t value); - -inline void pack_integer(std::string& buffer, std::int32_t value) { - pack_integer(buffer, std::int64_t(value)); -} +void pack_integer(std::string& buffer, std::int32_t value); void pack_double(std::string& buffer, double value); -void pack_string(std::string& buffer, std::string_view value); -void pack_array(std::string& buffer, std::size_t size); +Expected pack_string(std::string& buffer, std::string_view value); + +Expected pack_array(std::string& buffer, std::size_t size); // Append to the specified `buffer` a MessagePack encoded array having the // specified `values`, where for each element of `values` the specified // `pack_value` function appends the value. `pack_value` is invoked with two // arguments: the first is a reference to `buffer`, and the second is a -// reference to the current value. `pack_value` returns an `Expected`. -// If the return value is an error, then iteration is halted and the error is -// returned. Otherwise the non-error value is returned. +// reference to the current value. `pack_value` returns an `Expected`. If +// the return value is an error, then iteration is halted and the error is +// returned. If some other error occurs, then an error is returned. Otherwise, +// the non-error value is returned. template Expected pack_array(std::string& buffer, Iterable&& values, PackValue&& pack_value); -void pack_map(std::string& buffer, std::size_t size); +Expected pack_map(std::string& buffer, std::size_t size); // Append to the specified `buffer` a MessagePack encoded map consisting of the // specified `pairs`, where the first element of each pair is the name of the // map element, and the second element of each pair is some value that is // MessagePack encoded by the specified `pack_value` function. `pack_value` is // invoked with two arguments: the first is a reference to `buffer`, and the -// second is a reference to the current value. +// second is a reference to the current value. `pack_value` returns an +// `Expected`. If the return value is an error, then iteration is halted +// and the error is returned. If some other error occurs, then an error is +// returned. Otherwise, the non-error value is returned. template -void pack_map(std::string& buffer, const PairIterable& pairs, - PackValue&& pack_value); +Expected pack_map(std::string& buffer, const PairIterable& pairs, + PackValue&& pack_value); // Append to the specified `buffer` a MessagePack encoded map consisting of the // specified key value pairs. After the `buffer` argument, `pack_map` accepts @@ -64,21 +66,27 @@ void pack_map(std::string& buffer, const PairIterable& pairs, // key name of the corresponding map item. Second in each pair of arguments is // `pack_value`, a function that encodes the corresponding value. `pack_value` // is invoked with two arguments: the first is a reference to `buffer`, and the -// second is a reference to the current value. +// second is a reference to the current value. `pack_value` returns an +// `Expected`. If the return value is an error, then iteration is halted +// and the error is returned. If some other error occurs, then an error is +// returned. Otherwise, the non-error value is returned. template -void pack_map(std::string& buffer, Key&& key, PackValue&& pack_value, - Rest&&... rest); +Expected pack_map(std::string& buffer, Key&& key, PackValue&& pack_value, + Rest&&... rest); template -void pack_map_suffix(std::string& buffer, Key&& key, PackValue&& pack_value, - Rest&&... rest); -void pack_map_suffix(std::string& buffer); +Expected pack_map_suffix(std::string& buffer, Key&& key, + PackValue&& pack_value, Rest&&... rest); +Expected pack_map_suffix(std::string& buffer); template Expected pack_array(std::string& buffer, Iterable&& values, PackValue&& pack_value) { Expected result; - pack_array(buffer, std::size(values)); + result = pack_array(buffer, std::size(values)); + if (!result) { + return result; + } for (const auto& value : values) { result = pack_value(buffer, value); if (!result) { @@ -89,34 +97,63 @@ Expected pack_array(std::string& buffer, Iterable&& values, } template -void pack_map(std::string& buffer, const PairIterable& pairs, - PackValue&& pack_value) { - pack_map(buffer, std::size(pairs)); +Expected pack_map(std::string& buffer, const PairIterable& pairs, + PackValue&& pack_value) { + Expected result; + result = pack_map(buffer, std::size(pairs)); + if (!result) { + return result; + } for (const auto& [key, value] : pairs) { - pack_string(buffer, key); - pack_value(buffer, value); + result = pack_string(buffer, key); + if (!result) { + break; + } + result = pack_value(buffer, value); + if (!result) { + break; + } } + return result; } template -void pack_map(std::string& buffer, Key&& key, PackValue&& pack_value, - Rest&&... rest) { - pack_map(buffer, 1 + sizeof...(rest)); - pack_map_suffix(buffer, std::forward(key), - std::forward(pack_value), - std::forward(rest)...); +Expected pack_map(std::string& buffer, Key&& key, PackValue&& pack_value, + Rest&&... rest) { + Expected result; + result = pack_map(buffer, 1 + sizeof...(rest)); + if (!result) { + return result; + } + result = pack_map_suffix(buffer, std::forward(key), + std::forward(pack_value), + std::forward(rest)...); + return result; } template -void pack_map_suffix(std::string& buffer, Key&& key, PackValue&& pack_value, - Rest&&... rest) { - pack_string(buffer, key); - pack_value(buffer); - pack_map_suffix(buffer, std::forward(rest)...); +Expected pack_map_suffix(std::string& buffer, Key&& key, + PackValue&& pack_value, Rest&&... rest) { + Expected result; + result = pack_string(buffer, key); + if (!result) { + return result; + } + result = pack_value(buffer); + if (!result) { + return result; + } + result = pack_map_suffix(buffer, std::forward(rest)...); + return result; } -inline void pack_map_suffix(std::string&) { +inline Expected pack_map_suffix(std::string&) { // base case does nothing + return {}; +} + +inline void pack_integer(std::string& buffer, std::int32_t value) { + pack_integer(buffer, std::int64_t(value)); } } // namespace msgpack diff --git a/src/datadog/span_data.cpp b/src/datadog/span_data.cpp index 8056be0f..170f0c25 100644 --- a/src/datadog/span_data.cpp +++ b/src/datadog/span_data.cpp @@ -69,51 +69,58 @@ Expected msgpack_encode(std::string& destination, msgpack::pack_map( destination, "service", [&](auto& destination) { - msgpack::pack_string(destination, span.service); + return msgpack::pack_string(destination, span.service); }, "name", [&](auto& destination) { - msgpack::pack_string(destination, span.name); + return msgpack::pack_string(destination, span.name); }, "resource", [&](auto& destination) { - msgpack::pack_string(destination, span.resource); + return msgpack::pack_string(destination, span.resource); }, "trace_id", [&](auto& destination) { msgpack::pack_integer(destination, span.trace_id); + return Expected{}; }, "span_id", [&](auto& destination) { msgpack::pack_integer(destination, span.span_id); + return Expected{}; }, "parent_id", [&](auto& destination) { msgpack::pack_integer(destination, span.parent_id); + return Expected{}; }, "start", [&](auto& destination) { msgpack::pack_integer( destination, std::chrono::duration_cast( span.start.wall.time_since_epoch()) .count()); + return Expected{}; }, "duration", [&](auto& destination) { msgpack::pack_integer( destination, std::chrono::duration_cast(span.duration) .count()); + return Expected{}; }, "error", [&](auto& destination) { msgpack::pack_integer(destination, std::int32_t(span.error)); + return Expected{}; }, "meta", [&](auto& destination) { - msgpack::pack_map(destination, span.tags, + return msgpack::pack_map(destination, span.tags, [](std::string& destination, const auto& value) { - msgpack::pack_string(destination, value); + return msgpack::pack_string(destination, value); }); }, "metrics", [&](auto& destination) { - msgpack::pack_map(destination, span.numeric_tags, + return msgpack::pack_map(destination, span.numeric_tags, [](std::string& destination, const auto& value) { msgpack::pack_double(destination, value); + return Expected{}; }); }, "type", [&](auto& destination) { - msgpack::pack_string(destination, span.service_type); + return msgpack::pack_string(destination, span.service_type); }); // clang-format on From 9987e1afbdda99176349b7c164cb905035a608d8 Mon Sep 17 00:00:00 2001 From: David Goffredo Date: Fri, 30 Sep 2022 16:57:14 -0400 Subject: [PATCH 8/8] remove unnecessary try/catch blocks --- src/datadog/datadog_agent.cpp | 9 ++------- src/datadog/span_data.cpp | 6 +----- 2 files changed, 3 insertions(+), 12 deletions(-) diff --git a/src/datadog/datadog_agent.cpp b/src/datadog/datadog_agent.cpp index d553543c..ddbc102b 100644 --- a/src/datadog/datadog_agent.cpp +++ b/src/datadog/datadog_agent.cpp @@ -2,7 +2,6 @@ #include #include -#include #include #include #include @@ -31,25 +30,21 @@ HTTPClient::URL traces_endpoint(const HTTPClient::URL& agent_url) { Expected msgpack_encode( std::string& destination, - const std::vector>& spans) try { + const std::vector>& spans) { return msgpack::pack_array(destination, spans, [](auto& destination, const auto& span_ptr) { assert(span_ptr); return msgpack_encode(destination, *span_ptr); }); -} catch (const std::exception& error) { - return Error{Error::MESSAGEPACK_ENCODE_FAILURE, error.what()}; } Expected msgpack_encode( std::string& destination, - const std::vector& trace_chunks) try { + const std::vector& trace_chunks) { return msgpack::pack_array(destination, trace_chunks, [](auto& destination, const auto& chunk) { return msgpack_encode(destination, chunk.spans); }); -} catch (const std::exception& error) { - return Error{Error::MESSAGEPACK_ENCODE_FAILURE, error.what()}; } std::variant parse_agent_traces_response( diff --git a/src/datadog/span_data.cpp b/src/datadog/span_data.cpp index 170f0c25..2b701223 100644 --- a/src/datadog/span_data.cpp +++ b/src/datadog/span_data.cpp @@ -1,7 +1,6 @@ #include "span_data.h" #include -#include #include #include "error.h" @@ -63,8 +62,7 @@ void SpanData::apply_config(const SpanDefaults& defaults, } } -Expected msgpack_encode(std::string& destination, - const SpanData& span) try { +Expected msgpack_encode(std::string& destination, const SpanData& span) { // clang-format off msgpack::pack_map( destination, @@ -125,8 +123,6 @@ Expected msgpack_encode(std::string& destination, // clang-format on return std::nullopt; -} catch (const std::exception& error) { - return Error{Error::MESSAGEPACK_ENCODE_FAILURE, error.what()}; } } // namespace tracing