From 06b487119def15f0c49883f86c604f6e3b3398c9 Mon Sep 17 00:00:00 2001 From: Grant Limberg Date: Tue, 2 May 2023 11:16:55 -0700 Subject: [PATCH] More packet metrics (#1982) * found path negotation sends that weren't accounted for * Fix histogram so it will actually compile * Found more places for packet metrics --- .../core/include/prometheus/counter.h | 3 ++- .../core/include/prometheus/gauge.h | 1 + .../core/include/prometheus/histogram.h | 4 ++-- node/Bond.cpp | 2 ++ node/IncomingPacket.cpp | 14 ++++++++++++++ node/Membership.cpp | 1 + node/Metrics.cpp | 1 + node/Metrics.hpp | 2 +- node/Multicaster.cpp | 2 ++ node/Peer.cpp | 11 +++++++++++ node/Peer.hpp | 5 +++-- node/Switch.cpp | 1 + 12 files changed, 41 insertions(+), 6 deletions(-) diff --git a/ext/prometheus-cpp-lite-1.0/core/include/prometheus/counter.h b/ext/prometheus-cpp-lite-1.0/core/include/prometheus/counter.h index 35231888f..5d8053739 100644 --- a/ext/prometheus-cpp-lite-1.0/core/include/prometheus/counter.h +++ b/ext/prometheus-cpp-lite-1.0/core/include/prometheus/counter.h @@ -38,7 +38,8 @@ namespace prometheus { static const Metric::Type static_type = Metric::Type::Counter; Counter() : Metric (Metric::Type::Counter) {} ///< \brief Create a counter that starts at 0. - + Counter(const Counter &rhs) : Metric(static_type), value{rhs.value.load()} {} + // original API void Increment() { ///< \brief Increment the counter by 1. diff --git a/ext/prometheus-cpp-lite-1.0/core/include/prometheus/gauge.h b/ext/prometheus-cpp-lite-1.0/core/include/prometheus/gauge.h index f42308526..fcda14631 100644 --- a/ext/prometheus-cpp-lite-1.0/core/include/prometheus/gauge.h +++ b/ext/prometheus-cpp-lite-1.0/core/include/prometheus/gauge.h @@ -38,6 +38,7 @@ namespace prometheus { Gauge() : Metric (static_type) {} ///< \brief Create a gauge that starts at 0. Gauge(const Value value_) : Metric(static_type), value{ value_ } {} ///< \brief Create a gauge that starts at the given amount. + Gauge(const Gauge &rhs) : Metric(rhs.static_type), value{rhs.value.load()} {} // original API diff --git a/ext/prometheus-cpp-lite-1.0/core/include/prometheus/histogram.h b/ext/prometheus-cpp-lite-1.0/core/include/prometheus/histogram.h index 03cf461ea..ac558fa21 100644 --- a/ext/prometheus-cpp-lite-1.0/core/include/prometheus/histogram.h +++ b/ext/prometheus-cpp-lite-1.0/core/include/prometheus/histogram.h @@ -27,7 +27,7 @@ namespace prometheus { /// The class is thread-safe. No concurrent call to any API of this type causes /// a data race. template - class Histogram : Metric { + class Histogram : public Metric { using BucketBoundaries = std::vector; @@ -105,7 +105,7 @@ namespace prometheus { auto cumulative_count = 0ULL; metric.histogram.bucket.reserve(bucket_counts_.size()); for (std::size_t i{0}; i < bucket_counts_.size(); ++i) { - cumulative_count += static_cast(bucket_counts_[i].Value()); + cumulative_count += static_cast(bucket_counts_[i].Get()); auto bucket = ClientMetric::Bucket{}; bucket.cumulative_count = cumulative_count; bucket.upper_bound = (i == bucket_boundaries_.size() diff --git a/node/Bond.cpp b/node/Bond.cpp index 0eee7cff3..57551a747 100644 --- a/node/Bond.cpp +++ b/node/Bond.cpp @@ -795,6 +795,7 @@ void Bond::sendPATH_NEGOTIATION_REQUEST(void* tPtr, int pathIdx) Packet outp(_peer->_id.address(), RR->identity.address(), Packet::VERB_PATH_NEGOTIATION_REQUEST); outp.append(_localUtility); if (_paths[pathIdx].p->address()) { + Metrics::pkt_path_negotiation_request_out++; outp.armor(_peer->key(), false, _peer->aesKeysIfSupported()); RR->node->putPacket(tPtr, _paths[pathIdx].p->localSocket(), _paths[pathIdx].p->address(), outp.data(), outp.size()); _overheadBytes += outp.size(); @@ -841,6 +842,7 @@ void Bond::sendQOS_MEASUREMENT(void* tPtr, int pathIdx, int64_t localSocket, con } else { RR->sw->send(tPtr, outp, false); } + Metrics::pkt_qos_out++; _paths[pathIdx].packetsReceivedSinceLastQoS = 0; _paths[pathIdx].lastQoSMeasurement = now; _overheadBytes += outp.size(); diff --git a/node/IncomingPacket.cpp b/node/IncomingPacket.cpp index cc48caeb1..77dcf6b6a 100644 --- a/node/IncomingPacket.cpp +++ b/node/IncomingPacket.cpp @@ -410,6 +410,8 @@ bool IncomingPacket::_doHELLO(const RuntimeEnvironment *RR,void *tPtr,const bool outp.append((uint64_t)pid); outp.append((uint8_t)Packet::ERROR_IDENTITY_COLLISION); outp.armor(key,true,peer->aesKeysIfSupported()); + Metrics::pkt_error_out++; + Metrics::pkt_error_identity_collision_out++; _path->send(RR,tPtr,outp.data(),outp.size(),RR->node->now()); } else { RR->t->incomingPacketMessageAuthenticationFailure(tPtr,_path,pid,fromAddress,hops(),"invalid MAC"); @@ -567,6 +569,7 @@ bool IncomingPacket::_doHELLO(const RuntimeEnvironment *RR,void *tPtr,const bool outp.armor(peer->key(),true,peer->aesKeysIfSupported()); peer->recordOutgoingPacket(_path,outp.packetId(),outp.payloadLength(),outp.verb(),ZT_QOS_NO_FLOW,now); + Metrics::pkt_ok_out++; _path->send(RR,tPtr,outp.data(),outp.size(),now); peer->setRemoteVersion(protoVersion,vMajor,vMinor,vRevision); // important for this to go first so received() knows the version @@ -728,7 +731,9 @@ bool IncomingPacket::_doWHOIS(const RuntimeEnvironment *RR,void *tPtr,const Shar } if (count > 0) { + Metrics::pkt_ok_out++; outp.armor(peer->key(),true,peer->aesKeysIfSupported()); + Metrics::pkt_ok_out++; _path->send(RR,tPtr,outp.data(),outp.size(),RR->node->now()); } @@ -960,6 +965,7 @@ bool IncomingPacket::_doEXT_FRAME(const RuntimeEnvironment *RR,void *tPtr,const const int64_t now = RR->node->now(); outp.armor(peer->key(),true,peer->aesKeysIfSupported()); peer->recordOutgoingPacket(_path,outp.packetId(),outp.payloadLength(),outp.verb(),ZT_QOS_NO_FLOW,now); + Metrics::pkt_ok_out++; _path->send(RR,tPtr,outp.data(),outp.size(),RR->node->now()); } @@ -988,6 +994,7 @@ bool IncomingPacket::_doECHO(const RuntimeEnvironment *RR,void *tPtr,const Share } outp.armor(peer->key(),true,peer->aesKeysIfSupported()); peer->recordOutgoingPacket(_path,outp.packetId(),outp.payloadLength(),outp.verb(),ZT_QOS_NO_FLOW,now); + Metrics::pkt_ok_out++; _path->send(RR,tPtr,outp.data(),outp.size(),RR->node->now()); peer->received(tPtr,_path,hops(),pid,payloadLength(),Packet::VERB_ECHO,0,Packet::VERB_NOP,false,0,ZT_QOS_NO_FLOW); @@ -1182,6 +1189,8 @@ bool IncomingPacket::_doNETWORK_CONFIG_REQUEST(const RuntimeEnvironment *RR,void outp.append((unsigned char)Packet::ERROR_UNSUPPORTED_OPERATION); outp.append(nwid); outp.armor(peer->key(),true,peer->aesKeysIfSupported()); + Metrics::pkt_error_out++; + Metrics::pkt_error_unsupported_op_out++; _path->send(RR,tPtr,outp.data(),outp.size(),RR->node->now()); } @@ -1205,6 +1214,7 @@ bool IncomingPacket::_doNETWORK_CONFIG(const RuntimeEnvironment *RR,void *tPtr,c const int64_t now = RR->node->now(); outp.armor(peer->key(),true,peer->aesKeysIfSupported()); peer->recordOutgoingPacket(_path,outp.packetId(),outp.payloadLength(),outp.verb(),ZT_QOS_NO_FLOW,now); + Metrics::pkt_ok_out++; _path->send(RR,tPtr,outp.data(),outp.size(),RR->node->now()); } } @@ -1247,6 +1257,7 @@ bool IncomingPacket::_doMULTICAST_GATHER(const RuntimeEnvironment *RR,void *tPtr if (gatheredLocally > 0) { outp.armor(peer->key(),true,peer->aesKeysIfSupported()); peer->recordOutgoingPacket(_path,outp.packetId(),outp.payloadLength(),outp.verb(),ZT_QOS_NO_FLOW,now); + Metrics::pkt_ok_out++; _path->send(RR,tPtr,outp.data(),outp.size(),now); } } @@ -1350,6 +1361,7 @@ bool IncomingPacket::_doMULTICAST_FRAME(const RuntimeEnvironment *RR,void *tPtr, const int64_t now = RR->node->now(); outp.armor(peer->key(),true,peer->aesKeysIfSupported()); peer->recordOutgoingPacket(_path,outp.packetId(),outp.payloadLength(),outp.verb(),ZT_QOS_NO_FLOW,now); + Metrics::pkt_ok_out++; _path->send(RR,tPtr,outp.data(),outp.size(),RR->node->now()); } } @@ -1493,6 +1505,8 @@ void IncomingPacket::_sendErrorNeedCredentials(const RuntimeEnvironment *RR,void outp.append((uint8_t)Packet::ERROR_NEED_MEMBERSHIP_CERTIFICATE); outp.append(nwid); outp.armor(peer->key(),true,peer->aesKeysIfSupported()); + Metrics::pkt_error_out++; + Metrics::pkt_error_need_membership_cert_out++; _path->send(RR,tPtr,outp.data(),outp.size(),RR->node->now()); } diff --git a/node/Membership.cpp b/node/Membership.cpp index 26a89a271..1b317d94f 100644 --- a/node/Membership.cpp +++ b/node/Membership.cpp @@ -100,6 +100,7 @@ void Membership::pushCredentials(const RuntimeEnvironment *RR,void *tPtr,const i outp.compress(); RR->sw->send(tPtr,outp,true); + Metrics::pkt_network_credentials_out++; } _lastPushedCredentials = now; diff --git a/node/Metrics.cpp b/node/Metrics.cpp index d57bb2d00..c90331982 100644 --- a/node/Metrics.cpp +++ b/node/Metrics.cpp @@ -11,6 +11,7 @@ */ #include +#include namespace prometheus { namespace simpleapi { diff --git a/node/Metrics.hpp b/node/Metrics.hpp index f38bab6b9..7530e48c0 100644 --- a/node/Metrics.hpp +++ b/node/Metrics.hpp @@ -13,6 +13,7 @@ #define METRICS_H_ #include +#include namespace prometheus { namespace simpleapi { @@ -94,7 +95,6 @@ namespace ZeroTier { extern prometheus::simpleapi::counter_metric_t pkt_error_authentication_required_out; extern prometheus::simpleapi::counter_metric_t pkt_error_internal_server_error_out; - // Data Sent/Received Metrics extern prometheus::simpleapi::counter_metric_t udp_send; extern prometheus::simpleapi::counter_metric_t udp_recv; diff --git a/node/Multicaster.cpp b/node/Multicaster.cpp index cc26cddda..1306a53f2 100644 --- a/node/Multicaster.cpp +++ b/node/Multicaster.cpp @@ -200,6 +200,7 @@ void Multicaster::send( outp.compress(); } outp.armor(bestMulticastReplicator->key(),true,bestMulticastReplicator->aesKeysIfSupported()); + Metrics::pkt_multicast_frame_out++; bestMulticastReplicatorPath->send(RR,tPtr,outp.data(),outp.size(),now); return; } @@ -334,6 +335,7 @@ void Multicaster::send( } RR->node->expectReplyTo(outp.packetId()); RR->sw->send(tPtr,outp,true); + Metrics::pkt_multicast_gather_out++; } } diff --git a/node/Peer.cpp b/node/Peer.cpp index f9db88ea9..3aa2641cc 100644 --- a/node/Peer.cpp +++ b/node/Peer.cpp @@ -28,6 +28,11 @@ namespace ZeroTier { static unsigned char s_freeRandomByteCounter = 0; +char * peerIDString(const Identity &id) { + char out[16]; + return id.address().toString(out); +} + Peer::Peer(const RuntimeEnvironment *renv,const Identity &myIdentity,const Identity &peerIdentity) : RR(renv), _lastReceive(0), @@ -234,9 +239,11 @@ void Peer::received( ++p; } if (count) { + Metrics::pkt_push_direct_paths_out++; outp->setAt(ZT_PACKET_IDX_PAYLOAD,(uint16_t)count); outp->compress(); outp->armor(_key,true,aesKeysIfSupported()); + Metrics::pkt_push_direct_paths_out++; path->send(RR,tPtr,outp->data(),outp->size(),now); } delete outp; @@ -382,6 +389,7 @@ void Peer::introduce(void *const tPtr,const int64_t now,const SharedPtr &o outp.append(other->_paths[theirs].p->address().rawIpData(),4); } outp.armor(_key,true,aesKeysIfSupported()); + Metrics::pkt_rendezvous_out++; _paths[mine].p->send(RR,tPtr,outp.data(),outp.size(),now); } else { Packet outp(other->_id.address(),RR->identity.address(),Packet::VERB_RENDEZVOUS); @@ -396,6 +404,7 @@ void Peer::introduce(void *const tPtr,const int64_t now,const SharedPtr &o outp.append(_paths[mine].p->address().rawIpData(),4); } outp.armor(other->_key,true,other->aesKeysIfSupported()); + Metrics::pkt_rendezvous_out++; other->_paths[theirs].p->send(RR,tPtr,outp.data(),outp.size(),now); } ++alt; @@ -438,6 +447,7 @@ void Peer::sendHELLO(void *tPtr,const int64_t localSocket,const InetAddress &atA if (atAddress) { outp.armor(_key,false,nullptr); // false == don't encrypt full payload, but add MAC + Metrics::pkt_hello_out++; RR->node->expectReplyTo(outp.packetId()); RR->node->putPacket(tPtr,RR->node->lowBandwidthModeEnabled() ? localSocket : -1,atAddress,outp.data(),outp.size()); } else { @@ -451,6 +461,7 @@ void Peer::attemptToContactAt(void *tPtr,const int64_t localSocket,const InetAdd if ( (!sendFullHello) && (_vProto >= 5) && (!((_vMajor == 1)&&(_vMinor == 1)&&(_vRevision == 0))) ) { Packet outp(_id.address(),RR->identity.address(),Packet::VERB_ECHO); outp.armor(_key,true,aesKeysIfSupported()); + Metrics::pkt_echo_out++; RR->node->expectReplyTo(outp.packetId()); RR->node->putPacket(tPtr,localSocket,atAddress,outp.data(),outp.size()); } else { diff --git a/node/Peer.hpp b/node/Peer.hpp index 640ed1a2e..427e78a58 100644 --- a/node/Peer.hpp +++ b/node/Peer.hpp @@ -34,6 +34,7 @@ #include "Mutex.hpp" #include "Bond.hpp" #include "AES.hpp" +#include "Metrics.hpp" #define ZT_PEER_MAX_SERIALIZED_STATE_SIZE (sizeof(Peer) + 32 + (sizeof(Path) * 2)) @@ -50,7 +51,7 @@ class Peer friend class Bond; private: - Peer() {} // disabled to prevent bugs -- should not be constructed uninitialized + Peer() = delete; // disabled to prevent bugs -- should not be constructed uninitialized public: ~Peer() { @@ -320,7 +321,7 @@ public: } else { SharedPtr bp(getAppropriatePath(now,false)); if (bp) { - return bp->latency(); + return (unsigned int)bp->latency(); } return 0xffff; } diff --git a/node/Switch.cpp b/node/Switch.cpp index 5359180e4..5cbbeff85 100644 --- a/node/Switch.cpp +++ b/node/Switch.cpp @@ -102,6 +102,7 @@ void Switch::onRemotePacket(void *tPtr,const int64_t localSocket,const InetAddre _lastBeaconResponse = now; Packet outp(peer->address(),RR->identity.address(),Packet::VERB_NOP); outp.armor(peer->key(),true,peer->aesKeysIfSupported()); + Metrics::pkt_nop_out++; path->send(RR,tPtr,outp.data(),outp.size(),now); } }