From b225822678b267c189cf19707e3fb4a4cae413f1 Mon Sep 17 00:00:00 2001 From: Wesley Shillingford Date: Fri, 20 Mar 2020 10:04:53 +0000 Subject: [PATCH] Remove telemetry message versions (#2610) --- nano/core_test/node_telemetry.cpp | 109 +++++------------------------- nano/core_test/testutil.hpp | 10 +-- nano/node/common.cpp | 81 +++++++--------------- nano/node/common.hpp | 16 ++--- nano/node/telemetry.cpp | 38 ++--------- nano/slow_test/node.cpp | 2 +- 6 files changed, 61 insertions(+), 195 deletions(-) diff --git a/nano/core_test/node_telemetry.cpp b/nano/core_test/node_telemetry.cpp index 03a31255..4e5194aa 100644 --- a/nano/core_test/node_telemetry.cpp +++ b/nano/core_test/node_telemetry.cpp @@ -32,6 +32,7 @@ TEST (node_telemetry, consolidate_data) data.minor_version = 1; data.patch_version = 4; data.pre_release_version = 6; + data.maker = 2; data.timestamp = std::chrono::system_clock::time_point (std::chrono::milliseconds (time)); nano::telemetry_data data1; @@ -65,6 +66,7 @@ TEST (node_telemetry, consolidate_data) data2.minor_version = 1; data2.patch_version = 4; data2.pre_release_version = 6; + data2.maker = 2; data2.timestamp = std::chrono::system_clock::time_point (std::chrono::milliseconds (time)); std::vector all_data{ data, data1, data2 }; @@ -80,11 +82,11 @@ TEST (node_telemetry, consolidate_data) ASSERT_EQ (consolidated_telemetry_data.uptime, 6); ASSERT_EQ (consolidated_telemetry_data.genesis_block, nano::block_hash (4)); ASSERT_EQ (consolidated_telemetry_data.major_version, 20); - ASSERT_FALSE (consolidated_telemetry_data.minor_version.is_initialized ()); - ASSERT_FALSE (consolidated_telemetry_data.patch_version.is_initialized ()); - ASSERT_FALSE (consolidated_telemetry_data.pre_release_version.is_initialized ()); - ASSERT_FALSE (consolidated_telemetry_data.maker.is_initialized ()); - ASSERT_EQ (*consolidated_telemetry_data.timestamp, std::chrono::system_clock::time_point (std::chrono::milliseconds (time))); + ASSERT_EQ (consolidated_telemetry_data.minor_version, 1); + ASSERT_EQ (consolidated_telemetry_data.patch_version, 4); + ASSERT_EQ (consolidated_telemetry_data.pre_release_version, 6); + ASSERT_EQ (consolidated_telemetry_data.maker, 2); + ASSERT_EQ (consolidated_telemetry_data.timestamp, std::chrono::system_clock::time_point (std::chrono::milliseconds (time))); // Modify the metrics which may be either the mode or averages to ensure all are tested. all_data[2].bandwidth_cap = 53; @@ -99,99 +101,20 @@ TEST (node_telemetry, consolidate_data) auto consolidated_telemetry_data1 = nano::consolidate_telemetry_data (all_data); ASSERT_EQ (consolidated_telemetry_data1.major_version, 10); - ASSERT_EQ (*consolidated_telemetry_data1.minor_version, 2); - ASSERT_EQ (*consolidated_telemetry_data1.patch_version, 3); - ASSERT_EQ (*consolidated_telemetry_data1.pre_release_version, 6); - ASSERT_EQ (*consolidated_telemetry_data1.maker, 2); + ASSERT_EQ (consolidated_telemetry_data1.minor_version, 2); + ASSERT_EQ (consolidated_telemetry_data1.patch_version, 3); + ASSERT_EQ (consolidated_telemetry_data1.pre_release_version, 6); + ASSERT_EQ (consolidated_telemetry_data1.maker, 2); ASSERT_TRUE (consolidated_telemetry_data1.protocol_version == 11 || consolidated_telemetry_data1.protocol_version == 12 || consolidated_telemetry_data1.protocol_version == 13); ASSERT_EQ (consolidated_telemetry_data1.bandwidth_cap, 51); ASSERT_EQ (consolidated_telemetry_data1.genesis_block, nano::block_hash (3)); - ASSERT_EQ (*consolidated_telemetry_data1.timestamp, std::chrono::system_clock::time_point (std::chrono::milliseconds (time + 1))); + ASSERT_EQ (consolidated_telemetry_data1.timestamp, std::chrono::system_clock::time_point (std::chrono::milliseconds (time + 1))); // Test equality operator ASSERT_FALSE (consolidated_telemetry_data == consolidated_telemetry_data1); ASSERT_EQ (consolidated_telemetry_data, consolidated_telemetry_data); } -TEST (node_telemetry, consolidate_data_optional_data) -{ - auto time = 1582117035109; - nano::telemetry_data data; - data.major_version = 20; - data.minor_version = 1; - data.patch_version = 4; - data.pre_release_version = 6; - data.maker = 2; - data.timestamp = std::chrono::system_clock::time_point (std::chrono::milliseconds (time)); - - nano::telemetry_data missing_minor; - missing_minor.major_version = 20; - missing_minor.patch_version = 4; - missing_minor.timestamp = std::chrono::system_clock::time_point (std::chrono::milliseconds (time + 3)); - - nano::telemetry_data missing_all_optional; - - std::vector all_data{ data, data, missing_minor, missing_all_optional }; - auto consolidated_telemetry_data = nano::consolidate_telemetry_data (all_data); - ASSERT_EQ (consolidated_telemetry_data.major_version, 20); - ASSERT_EQ (*consolidated_telemetry_data.minor_version, 1); - ASSERT_EQ (*consolidated_telemetry_data.patch_version, 4); - ASSERT_EQ (*consolidated_telemetry_data.pre_release_version, 6); - ASSERT_EQ (*consolidated_telemetry_data.maker, 2); - ASSERT_EQ (*consolidated_telemetry_data.timestamp, std::chrono::system_clock::time_point (std::chrono::milliseconds (time + 1))); -} - -TEST (node_telemetry, serialize_deserialize_json_optional) -{ - nano::telemetry_data data; - data.minor_version = 1; - data.patch_version = 4; - data.pre_release_version = 6; - data.maker = 2; - data.timestamp = std::chrono::system_clock::time_point (100ms); - - nano::jsonconfig config; - data.serialize_json (config, false); - - uint8_t val; - ASSERT_FALSE (config.get ("minor_version", val).get_error ()); - ASSERT_EQ (val, 1); - ASSERT_FALSE (config.get ("patch_version", val).get_error ()); - ASSERT_EQ (val, 4); - ASSERT_FALSE (config.get ("pre_release_version", val).get_error ()); - ASSERT_EQ (val, 6); - ASSERT_FALSE (config.get ("maker", val).get_error ()); - ASSERT_EQ (val, 2); - uint64_t timestamp; - ASSERT_FALSE (config.get ("timestamp", timestamp).get_error ()); - ASSERT_EQ (timestamp, 100); - - nano::telemetry_data data1; - data1.deserialize_json (config, false); - ASSERT_EQ (*data1.minor_version, 1); - ASSERT_EQ (*data1.patch_version, 4); - ASSERT_EQ (*data1.pre_release_version, 6); - ASSERT_EQ (*data1.maker, 2); - ASSERT_EQ (*data1.timestamp, std::chrono::system_clock::time_point (100ms)); - - nano::telemetry_data no_optional_data; - nano::jsonconfig config1; - no_optional_data.serialize_json (config1, false); - ASSERT_FALSE (config1.get_optional ("minor_version").is_initialized ()); - ASSERT_FALSE (config1.get_optional ("patch_version").is_initialized ()); - ASSERT_FALSE (config1.get_optional ("pre_release_version").is_initialized ()); - ASSERT_FALSE (config1.get_optional ("maker").is_initialized ()); - ASSERT_FALSE (config1.get_optional ("timestamp").is_initialized ()); - - nano::telemetry_data no_optional_data1; - no_optional_data1.deserialize_json (config1, false); - ASSERT_FALSE (no_optional_data1.minor_version.is_initialized ()); - ASSERT_FALSE (no_optional_data1.patch_version.is_initialized ()); - ASSERT_FALSE (no_optional_data1.pre_release_version.is_initialized ()); - ASSERT_FALSE (no_optional_data1.maker.is_initialized ()); - ASSERT_FALSE (no_optional_data1.timestamp.is_initialized ()); -} - TEST (node_telemetry, consolidate_data_remove_outliers) { nano::telemetry_data data; @@ -431,10 +354,10 @@ TEST (node_telemetry, many_nodes) ASSERT_GE (data.bandwidth_cap, 100000); ASSERT_LT (data.bandwidth_cap, 100000 + system.nodes.size ()); ASSERT_EQ (data.major_version, nano::get_major_node_version ()); - ASSERT_EQ (*data.minor_version, nano::get_minor_node_version ()); - ASSERT_EQ (*data.patch_version, nano::get_patch_node_version ()); - ASSERT_EQ (*data.pre_release_version, nano::get_pre_release_node_version ()); - ASSERT_EQ (*data.maker, 0); + ASSERT_EQ (data.minor_version, nano::get_minor_node_version ()); + ASSERT_EQ (data.patch_version, nano::get_patch_node_version ()); + ASSERT_EQ (data.pre_release_version, nano::get_pre_release_node_version ()); + ASSERT_EQ (data.maker, 0); ASSERT_LT (data.uptime, 100); ASSERT_EQ (data.genesis_block, genesis.hash ()); } diff --git a/nano/core_test/testutil.hpp b/nano/core_test/testutil.hpp index 13753677..a31f6743 100644 --- a/nano/core_test/testutil.hpp +++ b/nano/core_test/testutil.hpp @@ -251,11 +251,11 @@ inline void compare_default_telemetry_response_data_excluding_signature (nano::t ASSERT_LT (telemetry_data_a.uptime, 100); ASSERT_EQ (telemetry_data_a.genesis_block, network_params_a.ledger.genesis_hash); ASSERT_EQ (telemetry_data_a.major_version, nano::get_major_node_version ()); - ASSERT_EQ (*telemetry_data_a.minor_version, nano::get_minor_node_version ()); - ASSERT_EQ (*telemetry_data_a.patch_version, nano::get_patch_node_version ()); - ASSERT_EQ (*telemetry_data_a.pre_release_version, nano::get_pre_release_node_version ()); - ASSERT_EQ (*telemetry_data_a.maker, 0); - ASSERT_GT (*telemetry_data_a.timestamp, std::chrono::system_clock::now () - std::chrono::seconds (100)); + ASSERT_EQ (telemetry_data_a.minor_version, nano::get_minor_node_version ()); + ASSERT_EQ (telemetry_data_a.patch_version, nano::get_patch_node_version ()); + ASSERT_EQ (telemetry_data_a.pre_release_version, nano::get_pre_release_node_version ()); + ASSERT_EQ (telemetry_data_a.maker, 0); + ASSERT_GT (telemetry_data_a.timestamp, std::chrono::system_clock::now () - std::chrono::seconds (100)); } inline void compare_default_telemetry_response_data (nano::telemetry_data const & telemetry_data_a, nano::network_params const & network_params_a, uint64_t bandwidth_limit_a, nano::keypair const & node_id_a) diff --git a/nano/node/common.cpp b/nano/node/common.cpp index 53f6f74e..a5e2f587 100644 --- a/nano/node/common.cpp +++ b/nano/node/common.cpp @@ -1171,29 +1171,17 @@ void nano::telemetry_data::deserialize (nano::stream & stream_a, uint16_t payloa read (stream_a, bandwidth_cap); read (stream_a, peer_count); read (stream_a, protocol_version); - read (stream_a, major_version); read (stream_a, uptime); read (stream_a, genesis_block.bytes); + read (stream_a, major_version); + read (stream_a, minor_version); + read (stream_a, patch_version); + read (stream_a, pre_release_version); + read (stream_a, maker); - if (payload_length_a > size_v0) - { - uint8_t out; - read (stream_a, out); - minor_version = out; - read (stream_a, out); - patch_version = out; - read (stream_a, out); - pre_release_version = out; - read (stream_a, out); - maker = out; - } - - if (payload_length_a > size_v1) - { - uint64_t timestamp_l; - read (stream_a, timestamp_l); - timestamp = std::chrono::system_clock::time_point (std::chrono::milliseconds (timestamp_l)); - } + uint64_t timestamp_l; + read (stream_a, timestamp_l); + timestamp = std::chrono::system_clock::time_point (std::chrono::milliseconds (timestamp_l)); } void nano::telemetry_data::serialize_without_signature (nano::stream & stream_a, uint16_t /* size_a */) const @@ -1206,14 +1194,14 @@ void nano::telemetry_data::serialize_without_signature (nano::stream & stream_a, write (stream_a, bandwidth_cap); write (stream_a, peer_count); write (stream_a, protocol_version); - write (stream_a, major_version); write (stream_a, uptime); write (stream_a, genesis_block.bytes); - write (stream_a, *minor_version); - write (stream_a, *patch_version); - write (stream_a, *pre_release_version); - write (stream_a, *maker); - write (stream_a, std::chrono::duration_cast (timestamp->time_since_epoch ()).count ()); + write (stream_a, major_version); + write (stream_a, minor_version); + write (stream_a, patch_version); + write (stream_a, pre_release_version); + write (stream_a, maker); + write (stream_a, std::chrono::duration_cast (timestamp.time_since_epoch ()).count ()); } void nano::telemetry_data::serialize (nano::stream & stream_a) const @@ -1239,26 +1227,11 @@ nano::error nano::telemetry_data::serialize_json (nano::jsonconfig & json, bool json.put ("uptime", uptime); json.put ("genesis_block", genesis_block.to_string ()); json.put ("major_version", major_version); - if (minor_version.is_initialized ()) - { - json.put ("minor_version", *minor_version); - } - if (patch_version.is_initialized ()) - { - json.put ("patch_version", *patch_version); - } - if (pre_release_version.is_initialized ()) - { - json.put ("pre_release_version", *pre_release_version); - } - if (maker.is_initialized ()) - { - json.put ("maker", *maker); - } - if (timestamp.is_initialized ()) - { - json.put ("timestamp", std::chrono::duration_cast (timestamp->time_since_epoch ()).count ()); - } + json.put ("minor_version", minor_version); + json.put ("patch_version", patch_version); + json.put ("pre_release_version", pre_release_version); + json.put ("maker", maker); + json.put ("timestamp", std::chrono::duration_cast (timestamp.time_since_epoch ()).count ()); return json.get_error (); } @@ -1305,16 +1278,12 @@ nano::error nano::telemetry_data::deserialize_json (nano::jsonconfig & json, boo } } json.get ("major_version", major_version); - minor_version = json.get_optional ("minor_version"); - patch_version = json.get_optional ("patch_version"); - pre_release_version = json.get_optional ("pre_release_version"); - maker = json.get_optional ("maker"); - auto timestamp_l = json.get_optional ("timestamp"); - if (timestamp_l.is_initialized ()) - { - timestamp = std::chrono::system_clock::time_point (std::chrono::milliseconds (*timestamp_l)); - } - + json.get ("minor_version", minor_version); + json.get ("patch_version", patch_version); + json.get ("pre_release_version", pre_release_version); + json.get ("maker", maker); + auto timestamp_l = json.get ("timestamp"); + timestamp = std::chrono::system_clock::time_point (std::chrono::milliseconds (timestamp_l)); return json.get_error (); } diff --git a/nano/node/common.hpp b/nano/node/common.hpp index faf35b0d..5213ac98 100644 --- a/nano/node/common.hpp +++ b/nano/node/common.hpp @@ -349,13 +349,13 @@ public: uint64_t uptime{ 0 }; uint32_t peer_count{ 0 }; uint8_t protocol_version{ 0 }; - uint8_t major_version{ 0 }; nano::block_hash genesis_block{ 0 }; - boost::optional minor_version; - boost::optional patch_version; - boost::optional pre_release_version; - boost::optional maker; // 0 for NF node - boost::optional timestamp; + uint8_t major_version{ 0 }; + uint8_t minor_version; + uint8_t patch_version; + uint8_t pre_release_version; + uint8_t maker; // 0 for NF node + std::chrono::system_clock::time_point timestamp; void serialize (nano::stream &) const; void deserialize (nano::stream &, uint16_t); @@ -366,9 +366,7 @@ public: bool operator== (nano::telemetry_data const &) const; bool operator!= (nano::telemetry_data const &) const; - static auto constexpr size_v0 = sizeof (signature) + sizeof (node_id) + sizeof (block_count) + sizeof (cemented_count) + sizeof (unchecked_count) + sizeof (account_count) + sizeof (bandwidth_cap) + sizeof (peer_count) + sizeof (protocol_version) + sizeof (uptime) + sizeof (genesis_block) + sizeof (major_version); - static auto constexpr size_v1 = size_v0 + sizeof (decltype (minor_version)::value_type) + sizeof (decltype (patch_version)::value_type) + sizeof (decltype (pre_release_version)::value_type) + sizeof (decltype (maker)::value_type); - static auto constexpr size = size_v1 + sizeof (uint64_t); + static auto constexpr size = sizeof (signature) + sizeof (node_id) + sizeof (block_count) + sizeof (cemented_count) + sizeof (unchecked_count) + sizeof (account_count) + sizeof (bandwidth_cap) + sizeof (peer_count) + sizeof (protocol_version) + sizeof (uptime) + sizeof (genesis_block) + sizeof (major_version) + sizeof (minor_version) + sizeof (patch_version) + sizeof (pre_release_version) + sizeof (maker) + sizeof (uint64_t); private: void serialize_without_signature (nano::stream &, uint16_t) const; diff --git a/nano/node/telemetry.cpp b/nano/node/telemetry.cpp index 016f6e98..01d28cb9 100644 --- a/nano/node/telemetry.cpp +++ b/nano/node/telemetry.cpp @@ -454,30 +454,9 @@ nano::telemetry_data nano::consolidate_telemetry_data (std::vector (telemetry_data.timestamp->time_since_epoch ()).count ()); - } - + ss << telemetry_data.major_version << "." << telemetry_data.minor_version << "." << telemetry_data.patch_version << "." << telemetry_data.pre_release_version << "." << telemetry_data.maker; ++vendor_versions[ss.str ()]; + timestamps.insert (std::chrono::duration_cast (telemetry_data.timestamp.time_since_epoch ()).count ()); ++protocol_versions[telemetry_data.protocol_version]; peer_counts.insert (telemetry_data.peer_count); unchecked_counts.insert (telemetry_data.unchecked_count); @@ -567,15 +546,12 @@ nano::telemetry_data nano::consolidate_telemetry_data (std::vector version_fragments; boost::split (version_fragments, version, boost::is_any_of (".")); - debug_assert (!version_fragments.empty () && version_fragments.size () <= 5); + debug_assert (version_fragments.size () == 5); consolidated_data.major_version = boost::lexical_cast (version_fragments.front ()); - if (version_fragments.size () == 5) - { - consolidated_data.minor_version = boost::lexical_cast (version_fragments[1]); - consolidated_data.patch_version = boost::lexical_cast (version_fragments[2]); - consolidated_data.pre_release_version = boost::lexical_cast (version_fragments[3]); - consolidated_data.maker = boost::lexical_cast (version_fragments[4]); - } + consolidated_data.minor_version = boost::lexical_cast (version_fragments[1]); + consolidated_data.patch_version = boost::lexical_cast (version_fragments[2]); + consolidated_data.pre_release_version = boost::lexical_cast (version_fragments[3]); + consolidated_data.maker = boost::lexical_cast (version_fragments[4]); return consolidated_data; } diff --git a/nano/slow_test/node.cpp b/nano/slow_test/node.cpp index 42a9cf87..5e4fff38 100644 --- a/nano/slow_test/node.cpp +++ b/nano/slow_test/node.cpp @@ -959,7 +959,7 @@ namespace transport auto peer = data.node->network.tcp_channels.channels[0].channel; data.node->telemetry->get_metrics_single_peer_async (peer, [&shared_data, &data, &node_data](nano::telemetry_data_response const & telemetry_data_response_a) { ASSERT_FALSE (telemetry_data_response_a.error); - callback_process (shared_data, data, node_data, *telemetry_data_response_a.telemetry_data.timestamp); + callback_process (shared_data, data, node_data, telemetry_data_response_a.telemetry_data.timestamp); }); } std::this_thread::sleep_for (1ms);