From 6790b72f6278a24034784a1378a33c32850e09f5 Mon Sep 17 00:00:00 2001 From: shripad621git Date: Wed, 17 Jan 2024 16:46:11 +0530 Subject: [PATCH 01/11] Set up the tracing to support counter macro and added the macro to none and multiplexed --- src/tracing/backend.h | 1 + src/tracing/macros.h | 1 + .../multiplexed/include/matter/tracing/macros_impl.h | 2 ++ src/tracing/none/include/matter/tracing/macros_impl.h | 2 ++ src/tracing/registry.cpp | 8 ++++++++ src/tracing/registry.h | 1 + 6 files changed, 15 insertions(+) diff --git a/src/tracing/backend.h b/src/tracing/backend.h index 7e04490e333ff7..36fda9ae33eea9 100644 --- a/src/tracing/backend.h +++ b/src/tracing/backend.h @@ -63,6 +63,7 @@ class Backend : public ::chip::IntrusiveListNodeBase<> /// Trace a zero-sized event virtual void TraceInstant(const char * label, const char * group) {} + virtual void TraceCounter(const char * label, const char * group) {} virtual void LogMessageSend(MessageSendInfo &) { TraceInstant("MessageSent", "Messaging"); } virtual void LogMessageReceived(MessageReceivedInfo &) { TraceInstant("MessageReceived", "Messaging"); } diff --git a/src/tracing/macros.h b/src/tracing/macros.h index 5f836e44d373bc..b456137b88a166 100644 --- a/src/tracing/macros.h +++ b/src/tracing/macros.h @@ -78,6 +78,7 @@ #define MATTER_TRACE_END(...) _MATTER_TRACE_DISABLE(__VA_ARGS__) #define MATTER_TRACE_INSTANT(...) _MATTER_TRACE_DISABLE(__VA_ARGS__) #define MATTER_TRACE_SCOPE(...) _MATTER_TRACE_DISABLE(__VA_ARGS__) +#define MATTER_TRACE_COUNTER(...) _MATTER_TRACE_DISABLE(__VA_ARGS__) #define MATTER_LOG_MESSAGE_SEND(...) _MATTER_TRACE_DISABLE(__VA_ARGS__) #define MATTER_LOG_MESSAGE_RECEIVED(...) _MATTER_TRACE_DISABLE(__VA_ARGS__) diff --git a/src/tracing/multiplexed/include/matter/tracing/macros_impl.h b/src/tracing/multiplexed/include/matter/tracing/macros_impl.h index e1f2adc56d989e..a55581b2ba4153 100644 --- a/src/tracing/multiplexed/include/matter/tracing/macros_impl.h +++ b/src/tracing/multiplexed/include/matter/tracing/macros_impl.h @@ -28,6 +28,8 @@ #define MATTER_TRACE_BEGIN(label, group) ::chip::Tracing::Internal::Begin(label, group) #define MATTER_TRACE_END(label, group) ::chip::Tracing::Internal::End(label, group) #define MATTER_TRACE_INSTANT(label, group) ::chip::Tracing::Internal::Instant(label, group) +#define MATTER_TRACE_COUNTER(label, group) ::chip::Tracing::Internal::Counter(label, group) + namespace chip { namespace Tracing { diff --git a/src/tracing/none/include/matter/tracing/macros_impl.h b/src/tracing/none/include/matter/tracing/macros_impl.h index 3f7f052b5cac0d..df5502b16d767a 100644 --- a/src/tracing/none/include/matter/tracing/macros_impl.h +++ b/src/tracing/none/include/matter/tracing/macros_impl.h @@ -31,3 +31,5 @@ #define MATTER_TRACE_END(...) _MATTER_TRACE_DISABLE(__VA_ARGS__) #define MATTER_TRACE_INSTANT(...) _MATTER_TRACE_DISABLE(__VA_ARGS__) #define MATTER_TRACE_SCOPE(...) _MATTER_TRACE_DISABLE(__VA_ARGS__) +#define MATTER_TRACE_COUNTER(...) _MATTER_TRACE_DISABLE(__VA_ARGS__) + diff --git a/src/tracing/registry.cpp b/src/tracing/registry.cpp index 9f7507aa9f75a6..80f2a7545a3f21 100644 --- a/src/tracing/registry.cpp +++ b/src/tracing/registry.cpp @@ -76,6 +76,14 @@ void Instant(const char * label, const char * group) } } +void Counter(const char * label, const char * group) +{ + for (auto & backend : gTracingBackends) + { + backend.TraceCounter(label, group); + } +} + void LogMessageSend(::chip::Tracing::MessageSendInfo & info) { for (auto & backend : gTracingBackends) diff --git a/src/tracing/registry.h b/src/tracing/registry.h index 853372585840fa..1dce9c13149f6d 100644 --- a/src/tracing/registry.h +++ b/src/tracing/registry.h @@ -76,6 +76,7 @@ namespace Internal { void Begin(const char * label, const char * group); void End(const char * label, const char * group); void Instant(const char * label, const char * group); +void Counter(const char * label, const char * group); void LogMessageSend(::chip::Tracing::MessageSendInfo & info); void LogMessageReceived(::chip::Tracing::MessageReceivedInfo & info); From 5557e542d7828bb95bef5adf0a40534be9364183 Mon Sep 17 00:00:00 2001 From: shripad621git Date: Wed, 17 Jan 2024 16:49:54 +0530 Subject: [PATCH 02/11] Added the support of counter macro for esp32 --- src/tracing/esp32_trace/BUILD.gn | 7 +++- src/tracing/esp32_trace/counter.cpp | 30 ++++++++++++++ src/tracing/esp32_trace/counter.h | 41 +++++++++++++++++++ src/tracing/esp32_trace/esp32_tracing.cpp | 5 +++ src/tracing/esp32_trace/esp32_tracing.h | 2 + .../include/matter/tracing/macros_impl.h | 1 + 6 files changed, 85 insertions(+), 1 deletion(-) create mode 100644 src/tracing/esp32_trace/counter.cpp create mode 100644 src/tracing/esp32_trace/counter.h diff --git a/src/tracing/esp32_trace/BUILD.gn b/src/tracing/esp32_trace/BUILD.gn index 7548894ab072ce..294cf1f5dc160b 100644 --- a/src/tracing/esp32_trace/BUILD.gn +++ b/src/tracing/esp32_trace/BUILD.gn @@ -29,6 +29,8 @@ static_library("backend") { output_dir = "${root_out_dir}/lib" sources = [ + "counter.cpp", + "counter.h", "esp32_tracing.cpp", "esp32_tracing.h", ] @@ -43,7 +45,10 @@ static_library("backend") { "${chip_root}/src/system", ] } - public_deps = [ "${chip_root}/src/tracing" ] + public_deps = [ + "${chip_root}/src/lib/core", + "${chip_root}/src/tracing", + ] } source_set("esp32_trace_tracing") { diff --git a/src/tracing/esp32_trace/counter.cpp b/src/tracing/esp32_trace/counter.cpp new file mode 100644 index 00000000000000..0fb03d082d1da7 --- /dev/null +++ b/src/tracing/esp32_trace/counter.cpp @@ -0,0 +1,30 @@ +#include "counter.h" + +namespace Insights{ +ESPInsightsCounter* ESPInsightsCounter::m_head = nullptr; + +ESPInsightsCounter* ESPInsightsCounter::GetInstance(const char* label, const char* group) { + + ESPInsightsCounter* current = m_head; + ESPInsightsCounter* previous = nullptr; + + while (current != nullptr) { + if (strcmp(current->label, label) == 0 && strcmp(current->group, group) == 0) { + current->instanceCount++; + return current; + } + previous = current; + current = current->m_next; + } + + ESPInsightsCounter* newInstance = new ESPInsightsCounter(label, group); + newInstance->m_next = m_head; + m_head = newInstance; + return newInstance; +} + +int ESPInsightsCounter::GetInstanceCount() const { + return instanceCount; +} + +} diff --git a/src/tracing/esp32_trace/counter.h b/src/tracing/esp32_trace/counter.h new file mode 100644 index 00000000000000..3c95015c32886d --- /dev/null +++ b/src/tracing/esp32_trace/counter.h @@ -0,0 +1,41 @@ +#include +#include +#include + +#define PATH1 "sys.cnt" +namespace Insights { +class ESPInsightsCounter +{ +private: + static ESPInsightsCounter * m_head; + char label[50]; + char group[50]; + int instanceCount; + ESPInsightsCounter * m_next; + bool registered = false; + + ESPInsightsCounter(const char * labelParam, const char * groupParam) : instanceCount(1), m_next(nullptr) + { + strncpy(label, labelParam, sizeof(label)); + strncpy(group, groupParam, sizeof(group)); + } + +public: + static ESPInsightsCounter * GetInstance(const char * label, const char * group); + + int GetInstanceCount() const; + + void ReportMetrics() + { + std::cout << "Trace instant: Label=" << label << ", Group=" << group << ", Instance Count=" << instanceCount << std::endl; + if (!registered) + { + esp_diag_metrics_register("SYS_CNT", label, label, PATH1, ESP_DIAG_DATA_TYPE_UINT); + registered = true; + } + std::cout << "Label ------" << label << "Count---" << instanceCount; + esp_diag_metrics_add_uint(label, instanceCount); + } +}; + +} // namespace Insights diff --git a/src/tracing/esp32_trace/esp32_tracing.cpp b/src/tracing/esp32_trace/esp32_tracing.cpp index 7a1313089703f2..1ab15e3e0126f4 100644 --- a/src/tracing/esp32_trace/esp32_tracing.cpp +++ b/src/tracing/esp32_trace/esp32_tracing.cpp @@ -18,6 +18,7 @@ #include "esp32_tracing.h" #include +#include "counter.h" #include #include #include @@ -148,6 +149,10 @@ void ESP32Backend::LogNodeDiscovered(NodeDiscoveredInfo & info) {} void ESP32Backend::LogNodeDiscoveryFailed(NodeDiscoveryFailedInfo & info) {} +void ESP32Backend::TraceCounter(const char * label, const char * group) +{ + ::Insights::ESPInsightsCounter::GetInstance(label, group)->ReportMetrics(); +} void ESP32Backend::TraceBegin(const char * label, const char * group) { HashValue hashValue = MurmurHash(group); diff --git a/src/tracing/esp32_trace/esp32_tracing.h b/src/tracing/esp32_trace/esp32_tracing.h index 7fe783d817d8d8..fb4125518281fa 100644 --- a/src/tracing/esp32_trace/esp32_tracing.h +++ b/src/tracing/esp32_trace/esp32_tracing.h @@ -28,6 +28,8 @@ class ESP32Backend : public ::chip::Tracing::Backend /// Trace a zero-sized event void TraceInstant(const char * label, const char * group) override; + void TraceCounter(const char * label, const char * group) override; + void LogMessageSend(MessageSendInfo &) override; void LogMessageReceived(MessageReceivedInfo &) override; diff --git a/src/tracing/esp32_trace/include/matter/tracing/macros_impl.h b/src/tracing/esp32_trace/include/matter/tracing/macros_impl.h index 4d8a8a2a214525..c439922f2cd5ff 100644 --- a/src/tracing/esp32_trace/include/matter/tracing/macros_impl.h +++ b/src/tracing/esp32_trace/include/matter/tracing/macros_impl.h @@ -28,6 +28,7 @@ #define MATTER_TRACE_BEGIN(label, group) ::chip::Tracing::Internal::Begin(label, group) #define MATTER_TRACE_END(label, group) ::chip::Tracing::Internal::End(label, group) #define MATTER_TRACE_INSTANT(label, group) ::chip::Tracing::Internal::Instant(label, group) +#define MATTER_TRACE_COUNTER(label, group) ::chip::Tracing::Internal::Counter(label, group) namespace chip { namespace Tracing { From 6def37ca3fde558a3fd82eb63ef8e9e450375a15 Mon Sep 17 00:00:00 2001 From: shripad621git Date: Wed, 17 Jan 2024 16:50:32 +0530 Subject: [PATCH 03/11] Added the support of counter macro in perfetto --- src/tracing/perfetto/BUILD.gn | 5 +++-- src/tracing/perfetto/include/matter/tracing/macros_impl.h | 7 +++++++ src/tracing/perfetto/perfetto_tracing.cpp | 3 +-- src/tracing/perfetto/perfetto_tracing.h | 1 + 4 files changed, 12 insertions(+), 4 deletions(-) diff --git a/src/tracing/perfetto/BUILD.gn b/src/tracing/perfetto/BUILD.gn index 64d4cd1d5d947b..fd1eaf3a7dc85b 100644 --- a/src/tracing/perfetto/BUILD.gn +++ b/src/tracing/perfetto/BUILD.gn @@ -25,7 +25,9 @@ config("tracing") { source_set("perfetto_tracing") { public = [ "include/matter/tracing/macros_impl.h" ] - public_deps = [ "${chip_root}/third_party/perfetto:sdk" ] + public_deps = [ + "${chip_root}/third_party/perfetto:sdk", + ] public_configs = [ ":tracing" ] } @@ -75,7 +77,6 @@ static_library("perfetto") { public_deps = [ ":perfetto_tracing", "${chip_root}/src/lib/address_resolve", - "${chip_root}/src/tracing", "${chip_root}/src/transport", "${chip_root}/third_party/perfetto:sdk", ] diff --git a/src/tracing/perfetto/include/matter/tracing/macros_impl.h b/src/tracing/perfetto/include/matter/tracing/macros_impl.h index 266ac69f5735a0..39b1c2697acf81 100644 --- a/src/tracing/perfetto/include/matter/tracing/macros_impl.h +++ b/src/tracing/perfetto/include/matter/tracing/macros_impl.h @@ -24,9 +24,16 @@ #include + PERFETTO_DEFINE_CATEGORIES(perfetto::Category("Matter").SetDescription("Matter trace events")); #define MATTER_TRACE_BEGIN(label, group) TRACE_EVENT_BEGIN("Matter", label, "class_name", group) #define MATTER_TRACE_END(label, group) TRACE_EVENT_END("Matter") #define MATTER_TRACE_INSTANT(label, group) TRACE_EVENT_INSTANT("Matter", label, "class_name", group) #define MATTER_TRACE_SCOPE(label, group) TRACE_EVENT("Matter", label, "class_name", group) + +#define MATTER_TRACE_COUNTER(label,group) \ + do { \ + static int count##_label = 0; \ + TRACE_COUNTER("Matter", perfetto::CounterTrack(label), ++count##_label); \ + } while(0) diff --git a/src/tracing/perfetto/perfetto_tracing.cpp b/src/tracing/perfetto/perfetto_tracing.cpp index 6d89dbbccc9723..b68ec33d33101d 100644 --- a/src/tracing/perfetto/perfetto_tracing.cpp +++ b/src/tracing/perfetto/perfetto_tracing.cpp @@ -21,10 +21,9 @@ #include #include #include -#include - #include #include +#include namespace chip { namespace Tracing { diff --git a/src/tracing/perfetto/perfetto_tracing.h b/src/tracing/perfetto/perfetto_tracing.h index 901165e35a548d..a7858c78305c67 100644 --- a/src/tracing/perfetto/perfetto_tracing.h +++ b/src/tracing/perfetto/perfetto_tracing.h @@ -17,6 +17,7 @@ */ #pragma once +#include #include #include From 70edb50b954615e3fa66e88567f3221a295452a0 Mon Sep 17 00:00:00 2001 From: shripad621git Date: Wed, 17 Jan 2024 16:51:35 +0530 Subject: [PATCH 04/11] Added support of counters for json backend and few traces --- src/lib/dnssd/Resolver_ImplMinimalMdns.cpp | 1 + src/protocols/secure_channel/CASESession.cpp | 6 ++++++ src/tracing/json/json_tracing.cpp | 22 +++++++++++++++++++- src/tracing/json/json_tracing.h | 4 ++++ 4 files changed, 32 insertions(+), 1 deletion(-) diff --git a/src/lib/dnssd/Resolver_ImplMinimalMdns.cpp b/src/lib/dnssd/Resolver_ImplMinimalMdns.cpp index 64461965367b13..4fd02f7d7da9ea 100644 --- a/src/lib/dnssd/Resolver_ImplMinimalMdns.cpp +++ b/src/lib/dnssd/Resolver_ImplMinimalMdns.cpp @@ -471,6 +471,7 @@ void MinMdnsResolver::OnMdnsPacketData(const BytesRange & data, const chip::Inet { MATTER_TRACE_SCOPE("Received MDNS Packet", "MinMdnsResolver"); + MATTER_TRACE_COUNTER("MDNSCount","MinMdnsResolver"); // Fill up any relevant data mPacketParser.ParseSrvRecords(data); mPacketParser.ParseNonSrvRecords(info->Interface, data); diff --git a/src/protocols/secure_channel/CASESession.cpp b/src/protocols/secure_channel/CASESession.cpp index 8573d1565686fe..ccb51c3117ab51 100644 --- a/src/protocols/secure_channel/CASESession.cpp +++ b/src/protocols/secure_channel/CASESession.cpp @@ -550,6 +550,7 @@ void CASESession::OnResponseTimeout(ExchangeContext * ec) MATTER_TRACE_SCOPE("OnResponseTimeout", "CASESession"); VerifyOrReturn(ec != nullptr, ChipLogError(SecureChannel, "CASESession::OnResponseTimeout was called by null exchange")); VerifyOrReturn(mExchangeCtxt == ec, ChipLogError(SecureChannel, "CASESession::OnResponseTimeout exchange doesn't match")); + MATTER_TRACE_COUNTER("casetimeout", "CASESession"); ChipLogError(SecureChannel, "CASESession timed out while waiting for a response from the peer. Current state was %u", to_underlying(mState)); // Discard the exchange so that Clear() doesn't try aborting it. The @@ -648,6 +649,7 @@ CHIP_ERROR CASESession::RecoverInitiatorIpk() CHIP_ERROR CASESession::SendSigma1() { MATTER_TRACE_SCOPE("SendSigma1", "CASESession"); + MATTER_TRACE_COUNTER("Sendsigmacnt","CASESession"); size_t data_len = TLV::EstimateStructOverhead(kSigmaParamRandomNumberSize, // initiatorRandom sizeof(uint16_t), // initiatorSessionId, kSHA256_Hash_Length, // destinationId @@ -749,6 +751,8 @@ CHIP_ERROR CASESession::SendSigma1() CHIP_ERROR CASESession::HandleSigma1_and_SendSigma2(System::PacketBufferHandle && msg) { MATTER_TRACE_SCOPE("HandleSigma1_and_SendSigma2", "CASESession"); + MATTER_TRACE_COUNTER("sigma1cnt", "CASESession"); + ReturnErrorOnFailure(HandleSigma1(std::move(msg))); return CHIP_NO_ERROR; @@ -970,6 +974,7 @@ CHIP_ERROR CASESession::SendSigma2Resume() CHIP_ERROR CASESession::SendSigma2() { MATTER_TRACE_SCOPE("SendSigma2", "CASESession"); + MATTER_TRACE_COUNTER("sigma2cnt", "CASESession"); VerifyOrReturnError(GetLocalSessionId().HasValue(), CHIP_ERROR_INCORRECT_STATE); VerifyOrReturnError(mFabricsTable != nullptr, CHIP_ERROR_INCORRECT_STATE); @@ -1565,6 +1570,7 @@ CHIP_ERROR CASESession::SendSigma3c(SendSigma3Data & data, CHIP_ERROR status) CHIP_ERROR CASESession::HandleSigma3a(System::PacketBufferHandle && msg) { MATTER_TRACE_SCOPE("HandleSigma3", "CASESession"); + MATTER_TRACE_COUNTER("sigma3cnt", "CASESession"); CHIP_ERROR err = CHIP_NO_ERROR; System::PacketBufferTLVReader tlvReader; TLV::TLVReader decryptedDataTlvReader; diff --git a/src/tracing/json/json_tracing.cpp b/src/tracing/json/json_tracing.cpp index 6c651b3190b485..c3ed8470bcab10 100644 --- a/src/tracing/json/json_tracing.cpp +++ b/src/tracing/json/json_tracing.cpp @@ -26,7 +26,6 @@ #include #include - #include #include @@ -276,6 +275,27 @@ void JsonBackend::TraceInstant(const char * label, const char * group) OutputValue(value); } +void JsonBackend::TraceCounter(const char * label, const char * group) +{ + std::string counterId = std::string(label); + if (mCounters.find(counterId) == mCounters.end()) + { + mCounters[counterId] = 1; + } + else + { + mCounters[counterId]++; + } + ::Json::Value value; + value["event"] = "TraceCounter"; + value["label"] = label; + value["group"] = group; + value["count"] = mCounters[counterId]; + + // Output the counter event + OutputValue(value); +} + void JsonBackend::LogMessageSend(MessageSendInfo & info) { ::Json::Value value; diff --git a/src/tracing/json/json_tracing.h b/src/tracing/json/json_tracing.h index 74451b705b73e0..9fd171036cf6d1 100644 --- a/src/tracing/json/json_tracing.h +++ b/src/tracing/json/json_tracing.h @@ -19,6 +19,7 @@ #include #include +#include namespace Json { class Value; @@ -50,6 +51,7 @@ class JsonBackend : public ::chip::Tracing::Backend void TraceBegin(const char * label, const char * group) override; void TraceEnd(const char * label, const char * group) override; void TraceInstant(const char * label, const char * group) override; + void TraceCounter(const char * label, const char * group) override; void LogMessageSend(MessageSendInfo &) override; void LogMessageReceived(MessageReceivedInfo &) override; void LogNodeLookup(NodeLookupInfo &) override; @@ -61,6 +63,8 @@ class JsonBackend : public ::chip::Tracing::Backend /// Does the actual write of the value void OutputValue(::Json::Value & value); + std::unordered_map mCounters; + // Output file if writing to a file. If closed, writing // to ChipLog* std::fstream mOutputFile; From d627144fd83688882a6f95dcfcdcfabe743e7c7c Mon Sep 17 00:00:00 2001 From: shripad621git Date: Thu, 18 Jan 2024 15:21:55 +0530 Subject: [PATCH 05/11] Restyled the code --- src/lib/dnssd/Resolver_ImplMinimalMdns.cpp | 2 +- src/protocols/secure_channel/CASESession.cpp | 2 +- src/tracing/esp32_trace/counter.cpp | 40 ++++++++++--------- src/tracing/esp32_trace/esp32_tracing.cpp | 2 +- src/tracing/json/json_tracing.cpp | 2 +- .../include/matter/tracing/macros_impl.h | 1 - .../none/include/matter/tracing/macros_impl.h | 1 - src/tracing/perfetto/BUILD.gn | 4 +- .../include/matter/tracing/macros_impl.h | 12 +++--- 9 files changed, 33 insertions(+), 33 deletions(-) diff --git a/src/lib/dnssd/Resolver_ImplMinimalMdns.cpp b/src/lib/dnssd/Resolver_ImplMinimalMdns.cpp index 4fd02f7d7da9ea..4c123ec5d7170a 100644 --- a/src/lib/dnssd/Resolver_ImplMinimalMdns.cpp +++ b/src/lib/dnssd/Resolver_ImplMinimalMdns.cpp @@ -471,7 +471,7 @@ void MinMdnsResolver::OnMdnsPacketData(const BytesRange & data, const chip::Inet { MATTER_TRACE_SCOPE("Received MDNS Packet", "MinMdnsResolver"); - MATTER_TRACE_COUNTER("MDNSCount","MinMdnsResolver"); + MATTER_TRACE_COUNTER("MDNSCount", "MinMdnsResolver"); // Fill up any relevant data mPacketParser.ParseSrvRecords(data); mPacketParser.ParseNonSrvRecords(info->Interface, data); diff --git a/src/protocols/secure_channel/CASESession.cpp b/src/protocols/secure_channel/CASESession.cpp index ccb51c3117ab51..e7f8d1e29a0658 100644 --- a/src/protocols/secure_channel/CASESession.cpp +++ b/src/protocols/secure_channel/CASESession.cpp @@ -649,7 +649,7 @@ CHIP_ERROR CASESession::RecoverInitiatorIpk() CHIP_ERROR CASESession::SendSigma1() { MATTER_TRACE_SCOPE("SendSigma1", "CASESession"); - MATTER_TRACE_COUNTER("Sendsigmacnt","CASESession"); + MATTER_TRACE_COUNTER("Sendsigmacnt", "CASESession"); size_t data_len = TLV::EstimateStructOverhead(kSigmaParamRandomNumberSize, // initiatorRandom sizeof(uint16_t), // initiatorSessionId, kSHA256_Hash_Length, // destinationId diff --git a/src/tracing/esp32_trace/counter.cpp b/src/tracing/esp32_trace/counter.cpp index 0fb03d082d1da7..a0ff880e142a04 100644 --- a/src/tracing/esp32_trace/counter.cpp +++ b/src/tracing/esp32_trace/counter.cpp @@ -1,30 +1,34 @@ #include "counter.h" -namespace Insights{ -ESPInsightsCounter* ESPInsightsCounter::m_head = nullptr; +namespace Insights { +ESPInsightsCounter * ESPInsightsCounter::m_head = nullptr; -ESPInsightsCounter* ESPInsightsCounter::GetInstance(const char* label, const char* group) { +ESPInsightsCounter * ESPInsightsCounter::GetInstance(const char * label, const char * group) +{ - ESPInsightsCounter* current = m_head; - ESPInsightsCounter* previous = nullptr; + ESPInsightsCounter * current = m_head; + ESPInsightsCounter * previous = nullptr; - while (current != nullptr) { - if (strcmp(current->label, label) == 0 && strcmp(current->group, group) == 0) { - current->instanceCount++; - return current; - } - previous = current; - current = current->m_next; + while (current != nullptr) + { + if (strcmp(current->label, label) == 0 && strcmp(current->group, group) == 0) + { + current->instanceCount++; + return current; } + previous = current; + current = current->m_next; + } - ESPInsightsCounter* newInstance = new ESPInsightsCounter(label, group); - newInstance->m_next = m_head; - m_head = newInstance; - return newInstance; + ESPInsightsCounter * newInstance = new ESPInsightsCounter(label, group); + newInstance->m_next = m_head; + m_head = newInstance; + return newInstance; } -int ESPInsightsCounter::GetInstanceCount() const { +int ESPInsightsCounter::GetInstanceCount() const +{ return instanceCount; } -} +} // namespace Insights diff --git a/src/tracing/esp32_trace/esp32_tracing.cpp b/src/tracing/esp32_trace/esp32_tracing.cpp index 1ab15e3e0126f4..68eb42157c18ef 100644 --- a/src/tracing/esp32_trace/esp32_tracing.cpp +++ b/src/tracing/esp32_trace/esp32_tracing.cpp @@ -17,8 +17,8 @@ */ #include "esp32_tracing.h" -#include #include "counter.h" +#include #include #include #include diff --git a/src/tracing/json/json_tracing.cpp b/src/tracing/json/json_tracing.cpp index c3ed8470bcab10..07b929670ee073 100644 --- a/src/tracing/json/json_tracing.cpp +++ b/src/tracing/json/json_tracing.cpp @@ -25,8 +25,8 @@ #include #include -#include #include +#include #include diff --git a/src/tracing/multiplexed/include/matter/tracing/macros_impl.h b/src/tracing/multiplexed/include/matter/tracing/macros_impl.h index a55581b2ba4153..90329e477b9ec2 100644 --- a/src/tracing/multiplexed/include/matter/tracing/macros_impl.h +++ b/src/tracing/multiplexed/include/matter/tracing/macros_impl.h @@ -30,7 +30,6 @@ #define MATTER_TRACE_INSTANT(label, group) ::chip::Tracing::Internal::Instant(label, group) #define MATTER_TRACE_COUNTER(label, group) ::chip::Tracing::Internal::Counter(label, group) - namespace chip { namespace Tracing { diff --git a/src/tracing/none/include/matter/tracing/macros_impl.h b/src/tracing/none/include/matter/tracing/macros_impl.h index df5502b16d767a..3119dce2a2b49f 100644 --- a/src/tracing/none/include/matter/tracing/macros_impl.h +++ b/src/tracing/none/include/matter/tracing/macros_impl.h @@ -32,4 +32,3 @@ #define MATTER_TRACE_INSTANT(...) _MATTER_TRACE_DISABLE(__VA_ARGS__) #define MATTER_TRACE_SCOPE(...) _MATTER_TRACE_DISABLE(__VA_ARGS__) #define MATTER_TRACE_COUNTER(...) _MATTER_TRACE_DISABLE(__VA_ARGS__) - diff --git a/src/tracing/perfetto/BUILD.gn b/src/tracing/perfetto/BUILD.gn index fd1eaf3a7dc85b..e2743a4ed9b4d9 100644 --- a/src/tracing/perfetto/BUILD.gn +++ b/src/tracing/perfetto/BUILD.gn @@ -25,9 +25,7 @@ config("tracing") { source_set("perfetto_tracing") { public = [ "include/matter/tracing/macros_impl.h" ] - public_deps = [ - "${chip_root}/third_party/perfetto:sdk", - ] + public_deps = [ "${chip_root}/third_party/perfetto:sdk" ] public_configs = [ ":tracing" ] } diff --git a/src/tracing/perfetto/include/matter/tracing/macros_impl.h b/src/tracing/perfetto/include/matter/tracing/macros_impl.h index 39b1c2697acf81..74cfbd20f33304 100644 --- a/src/tracing/perfetto/include/matter/tracing/macros_impl.h +++ b/src/tracing/perfetto/include/matter/tracing/macros_impl.h @@ -24,7 +24,6 @@ #include - PERFETTO_DEFINE_CATEGORIES(perfetto::Category("Matter").SetDescription("Matter trace events")); #define MATTER_TRACE_BEGIN(label, group) TRACE_EVENT_BEGIN("Matter", label, "class_name", group) @@ -32,8 +31,9 @@ PERFETTO_DEFINE_CATEGORIES(perfetto::Category("Matter").SetDescription("Matter t #define MATTER_TRACE_INSTANT(label, group) TRACE_EVENT_INSTANT("Matter", label, "class_name", group) #define MATTER_TRACE_SCOPE(label, group) TRACE_EVENT("Matter", label, "class_name", group) -#define MATTER_TRACE_COUNTER(label,group) \ - do { \ - static int count##_label = 0; \ - TRACE_COUNTER("Matter", perfetto::CounterTrack(label), ++count##_label); \ - } while(0) +#define MATTER_TRACE_COUNTER(label, group) \ + do \ + { \ + static int count##_label = 0; \ + TRACE_COUNTER("Matter", perfetto::CounterTrack(label), ++count##_label); \ + } while (0) From 14a86443c8748f28e6c464c1c14f0b0d7d085c08 Mon Sep 17 00:00:00 2001 From: shripad621git Date: Fri, 19 Jan 2024 11:58:52 +0530 Subject: [PATCH 06/11] Added few counter traces --- .../wifi-network-diagnostics-server.cpp | 1 - src/lib/dnssd/Resolver_ImplMinimalMdns.cpp | 1 - src/protocols/secure_channel/CASEServer.cpp | 2 +- src/protocols/secure_channel/CASESession.cpp | 11 +++++------ src/protocols/secure_channel/PASESession.cpp | 5 +++++ 5 files changed, 11 insertions(+), 9 deletions(-) diff --git a/src/app/clusters/wifi-network-diagnostics-server/wifi-network-diagnostics-server.cpp b/src/app/clusters/wifi-network-diagnostics-server/wifi-network-diagnostics-server.cpp index bd02f36d731cf2..d94059ed8299a8 100644 --- a/src/app/clusters/wifi-network-diagnostics-server/wifi-network-diagnostics-server.cpp +++ b/src/app/clusters/wifi-network-diagnostics-server/wifi-network-diagnostics-server.cpp @@ -245,7 +245,6 @@ class WiFiDiagnosticsDelegate : public DeviceLayer::WiFiDiagnosticsDelegate { MATTER_TRACE_SCOPE("OnDisconnectionDetected", "WiFiDiagnosticsDelegate"); ChipLogProgress(Zcl, "WiFiDiagnosticsDelegate: OnDisconnectionDetected"); - for (auto endpoint : EnabledEndpointsWithServerCluster(WiFiNetworkDiagnostics::Id)) { // If WiFi Network Diagnostics cluster is implemented on this endpoint diff --git a/src/lib/dnssd/Resolver_ImplMinimalMdns.cpp b/src/lib/dnssd/Resolver_ImplMinimalMdns.cpp index 4c123ec5d7170a..64461965367b13 100644 --- a/src/lib/dnssd/Resolver_ImplMinimalMdns.cpp +++ b/src/lib/dnssd/Resolver_ImplMinimalMdns.cpp @@ -471,7 +471,6 @@ void MinMdnsResolver::OnMdnsPacketData(const BytesRange & data, const chip::Inet { MATTER_TRACE_SCOPE("Received MDNS Packet", "MinMdnsResolver"); - MATTER_TRACE_COUNTER("MDNSCount", "MinMdnsResolver"); // Fill up any relevant data mPacketParser.ParseSrvRecords(data); mPacketParser.ParseNonSrvRecords(info->Interface, data); diff --git a/src/protocols/secure_channel/CASEServer.cpp b/src/protocols/secure_channel/CASEServer.cpp index d701d8c568c584..47a92621f5078e 100644 --- a/src/protocols/secure_channel/CASEServer.cpp +++ b/src/protocols/secure_channel/CASEServer.cpp @@ -182,7 +182,7 @@ void CASEServer::OnSessionEstablishmentError(CHIP_ERROR err) { MATTER_TRACE_SCOPE("OnSessionEstablishmentError", "CASEServer"); ChipLogError(Inet, "CASE Session establishment failed: %" CHIP_ERROR_FORMAT, err.Format()); - + MATTER_TRACE_SCOPE("CASEFail", "CASESession"); PrepareForSessionEstablishment(); } diff --git a/src/protocols/secure_channel/CASESession.cpp b/src/protocols/secure_channel/CASESession.cpp index e7f8d1e29a0658..5c6d7c257896c1 100644 --- a/src/protocols/secure_channel/CASESession.cpp +++ b/src/protocols/secure_channel/CASESession.cpp @@ -550,9 +550,9 @@ void CASESession::OnResponseTimeout(ExchangeContext * ec) MATTER_TRACE_SCOPE("OnResponseTimeout", "CASESession"); VerifyOrReturn(ec != nullptr, ChipLogError(SecureChannel, "CASESession::OnResponseTimeout was called by null exchange")); VerifyOrReturn(mExchangeCtxt == ec, ChipLogError(SecureChannel, "CASESession::OnResponseTimeout exchange doesn't match")); - MATTER_TRACE_COUNTER("casetimeout", "CASESession"); ChipLogError(SecureChannel, "CASESession timed out while waiting for a response from the peer. Current state was %u", to_underlying(mState)); + MATTER_TRACE_COUNTER("CASETimeout", "CASESession"); // Discard the exchange so that Clear() doesn't try aborting it. The // exchange will handle that. DiscardExchange(); @@ -649,7 +649,6 @@ CHIP_ERROR CASESession::RecoverInitiatorIpk() CHIP_ERROR CASESession::SendSigma1() { MATTER_TRACE_SCOPE("SendSigma1", "CASESession"); - MATTER_TRACE_COUNTER("Sendsigmacnt", "CASESession"); size_t data_len = TLV::EstimateStructOverhead(kSigmaParamRandomNumberSize, // initiatorRandom sizeof(uint16_t), // initiatorSessionId, kSHA256_Hash_Length, // destinationId @@ -751,8 +750,6 @@ CHIP_ERROR CASESession::SendSigma1() CHIP_ERROR CASESession::HandleSigma1_and_SendSigma2(System::PacketBufferHandle && msg) { MATTER_TRACE_SCOPE("HandleSigma1_and_SendSigma2", "CASESession"); - MATTER_TRACE_COUNTER("sigma1cnt", "CASESession"); - ReturnErrorOnFailure(HandleSigma1(std::move(msg))); return CHIP_NO_ERROR; @@ -848,6 +845,7 @@ CHIP_ERROR CASESession::HandleSigma1(System::PacketBufferHandle && msg) ByteSpan initiatorRandom; ChipLogProgress(SecureChannel, "Received Sigma1 msg"); + MATTER_TRACE_COUNTER("Sigma1", "CASESession"); bool sessionResumptionRequested = false; ByteSpan resumptionId; @@ -974,7 +972,6 @@ CHIP_ERROR CASESession::SendSigma2Resume() CHIP_ERROR CASESession::SendSigma2() { MATTER_TRACE_SCOPE("SendSigma2", "CASESession"); - MATTER_TRACE_COUNTER("sigma2cnt", "CASESession"); VerifyOrReturnError(GetLocalSessionId().HasValue(), CHIP_ERROR_INCORRECT_STATE); VerifyOrReturnError(mFabricsTable != nullptr, CHIP_ERROR_INCORRECT_STATE); @@ -1105,6 +1102,7 @@ CHIP_ERROR CASESession::SendSigma2() mState = State::kSentSigma2; ChipLogProgress(SecureChannel, "Sent Sigma2 msg"); + MATTER_TRACE_COUNTER("Sigma2", "CASESession"); return CHIP_NO_ERROR; } @@ -1121,6 +1119,7 @@ CHIP_ERROR CASESession::HandleSigma2Resume(System::PacketBufferHandle && msg) uint32_t decodeTagIdSeq = 0; ChipLogDetail(SecureChannel, "Received Sigma2Resume msg"); + MATTER_TRACE_COUNTER("Sigma2Resume", "CASESession"); uint8_t sigma2ResumeMIC[CHIP_CRYPTO_AEAD_MIC_LENGTH_BYTES]; @@ -1570,7 +1569,6 @@ CHIP_ERROR CASESession::SendSigma3c(SendSigma3Data & data, CHIP_ERROR status) CHIP_ERROR CASESession::HandleSigma3a(System::PacketBufferHandle && msg) { MATTER_TRACE_SCOPE("HandleSigma3", "CASESession"); - MATTER_TRACE_COUNTER("sigma3cnt", "CASESession"); CHIP_ERROR err = CHIP_NO_ERROR; System::PacketBufferTLVReader tlvReader; TLV::TLVReader decryptedDataTlvReader; @@ -1591,6 +1589,7 @@ CHIP_ERROR CASESession::HandleSigma3a(System::PacketBufferHandle && msg) uint8_t msg_salt[kIPKSize + kSHA256_Hash_Length]; ChipLogProgress(SecureChannel, "Received Sigma3 msg"); + MATTER_TRACE_COUNTER("Sigma3", "CASESession"); auto helper = WorkHelper::Create(*this, &HandleSigma3b, &CASESession::HandleSigma3c); VerifyOrExit(helper, err = CHIP_ERROR_NO_MEMORY); diff --git a/src/protocols/secure_channel/PASESession.cpp b/src/protocols/secure_channel/PASESession.cpp index 67da1d103ebf83..7cb8b821c7505f 100644 --- a/src/protocols/secure_channel/PASESession.cpp +++ b/src/protocols/secure_channel/PASESession.cpp @@ -251,6 +251,7 @@ void PASESession::OnResponseTimeout(ExchangeContext * ec) // If we were waiting for something, mNextExpectedMsg had better have a value. ChipLogError(SecureChannel, "PASESession timed out while waiting for a response from the peer. Expected message type was %u", to_underlying(mNextExpectedMsg.Value())); + MATTER_TRACE_COUNTER("PASETimeout", "PASESession"); // Discard the exchange so that Clear() doesn't try closing it. The // exchange will handle that. DiscardExchange(); @@ -572,6 +573,7 @@ CHIP_ERROR PASESession::HandleMsg1_and_SendMsg2(System::PacketBufferHandle && ms size_t verifier_len = kMAX_Hash_Length; ChipLogDetail(SecureChannel, "Received spake2p msg1"); + MATTER_TRACE_SCOPE("Pake1", "PASESession"); System::PacketBufferTLVReader tlvReader; TLV::TLVType containerType = TLV::kTLVType_Structure; @@ -620,6 +622,7 @@ CHIP_ERROR PASESession::HandleMsg1_and_SendMsg2(System::PacketBufferHandle && ms } ChipLogDetail(SecureChannel, "Sent spake2p msg2"); + MATTER_TRACE_COUNTER("Pake2", "PASESession"); exit: @@ -711,6 +714,7 @@ CHIP_ERROR PASESession::HandleMsg3(System::PacketBufferHandle && msg) CHIP_ERROR err = CHIP_NO_ERROR; ChipLogDetail(SecureChannel, "Received spake2p msg3"); + MATTER_TRACE_COUNTER("Pake3", "PASESession"); mNextExpectedMsg.ClearValue(); @@ -871,6 +875,7 @@ CHIP_ERROR PASESession::OnMessageReceived(ExchangeContext * exchange, const Payl DiscardExchange(); Clear(); ChipLogError(SecureChannel, "Failed during PASE session setup: %" CHIP_ERROR_FORMAT, err.Format()); + MATTER_TRACE_COUNTER("PASEFail", "PASESession"); // Do this last in case the delegate frees us. NotifySessionEstablishmentError(err); } From 6882eb3f150059f63ad063a27c62bbc0787a82dd Mon Sep 17 00:00:00 2001 From: shripad621git Date: Fri, 19 Jan 2024 17:46:30 +0530 Subject: [PATCH 07/11] Refactored some code --- scripts/tools/check_includes_config.py | 2 +- src/tracing/esp32_trace/counter.cpp | 30 ++++++++++++++++++----- src/tracing/esp32_trace/counter.h | 29 +++++++++++++++++----- src/tracing/esp32_trace/esp32_tracing.cpp | 4 +-- 4 files changed, 50 insertions(+), 15 deletions(-) diff --git a/scripts/tools/check_includes_config.py b/scripts/tools/check_includes_config.py index abe2c0ed42f5af..6e72115b758485 100644 --- a/scripts/tools/check_includes_config.py +++ b/scripts/tools/check_includes_config.py @@ -158,7 +158,7 @@ # Library meant for non-embedded 'src/tracing/json/json_tracing.cpp': {'string', 'sstream'}, - 'src/tracing/json/json_tracing.h': {'fstream'}, + 'src/tracing/json/json_tracing.h': {'fstream', 'unordered_map'}, # Not intended for embedded clients 'src/lib/support/jsontlv/JsonToTlv.cpp': {'sstream'}, diff --git a/src/tracing/esp32_trace/counter.cpp b/src/tracing/esp32_trace/counter.cpp index a0ff880e142a04..8d449c6e205969 100644 --- a/src/tracing/esp32_trace/counter.cpp +++ b/src/tracing/esp32_trace/counter.cpp @@ -1,12 +1,30 @@ -#include "counter.h" +/* + * + * Copyright (c) 2023 Project CHIP Authors + * All rights reserved. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +#include namespace Insights { -ESPInsightsCounter * ESPInsightsCounter::m_head = nullptr; +ESPInsightsCounter * ESPInsightsCounter::mHead = nullptr; ESPInsightsCounter * ESPInsightsCounter::GetInstance(const char * label, const char * group) { - ESPInsightsCounter * current = m_head; + ESPInsightsCounter * current = mHead; ESPInsightsCounter * previous = nullptr; while (current != nullptr) @@ -17,12 +35,12 @@ ESPInsightsCounter * ESPInsightsCounter::GetInstance(const char * label, const c return current; } previous = current; - current = current->m_next; + current = current->mNext; } ESPInsightsCounter * newInstance = new ESPInsightsCounter(label, group); - newInstance->m_next = m_head; - m_head = newInstance; + newInstance->mNext = mHead; + mHead = newInstance; return newInstance; } diff --git a/src/tracing/esp32_trace/counter.h b/src/tracing/esp32_trace/counter.h index 3c95015c32886d..f306023421fc50 100644 --- a/src/tracing/esp32_trace/counter.h +++ b/src/tracing/esp32_trace/counter.h @@ -1,5 +1,23 @@ +/* + * + * Copyright (c) 2023 Project CHIP Authors + * All rights reserved. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + #include -#include +#include #include #define PATH1 "sys.cnt" @@ -7,14 +25,14 @@ namespace Insights { class ESPInsightsCounter { private: - static ESPInsightsCounter * m_head; + static ESPInsightsCounter * mHead; char label[50]; char group[50]; int instanceCount; - ESPInsightsCounter * m_next; + ESPInsightsCounter * mNext; bool registered = false; - ESPInsightsCounter(const char * labelParam, const char * groupParam) : instanceCount(1), m_next(nullptr) + ESPInsightsCounter(const char * labelParam, const char * groupParam) : instanceCount(1), mNext(nullptr) { strncpy(label, labelParam, sizeof(label)); strncpy(group, groupParam, sizeof(group)); @@ -27,13 +45,12 @@ class ESPInsightsCounter void ReportMetrics() { - std::cout << "Trace instant: Label=" << label << ", Group=" << group << ", Instance Count=" << instanceCount << std::endl; if (!registered) { esp_diag_metrics_register("SYS_CNT", label, label, PATH1, ESP_DIAG_DATA_TYPE_UINT); registered = true; } - std::cout << "Label ------" << label << "Count---" << instanceCount; + ESP_LOGI("Mtr", "Label = %s Count = %d", label, instanceCount); esp_diag_metrics_add_uint(label, instanceCount); } }; diff --git a/src/tracing/esp32_trace/esp32_tracing.cpp b/src/tracing/esp32_trace/esp32_tracing.cpp index 68eb42157c18ef..3bcd6ec5842479 100644 --- a/src/tracing/esp32_trace/esp32_tracing.cpp +++ b/src/tracing/esp32_trace/esp32_tracing.cpp @@ -16,14 +16,14 @@ * limitations under the License. */ -#include "esp32_tracing.h" -#include "counter.h" #include #include #include #include #include #include +#include +#include namespace chip { namespace Tracing { From f56c30df0ecb58f8dbd383278a6e79fc81273102 Mon Sep 17 00:00:00 2001 From: shripad621git Date: Thu, 1 Feb 2024 11:56:38 +0530 Subject: [PATCH 08/11] Addressed Review Comments --- src/tracing/esp32_trace/counter.cpp | 18 ++++++++++----- src/tracing/esp32_trace/counter.h | 22 +++++++++++-------- src/tracing/perfetto/BUILD.gn | 1 + .../include/matter/tracing/macros_impl.h | 4 ++-- 4 files changed, 28 insertions(+), 17 deletions(-) diff --git a/src/tracing/esp32_trace/counter.cpp b/src/tracing/esp32_trace/counter.cpp index 8d449c6e205969..65826c656057ae 100644 --- a/src/tracing/esp32_trace/counter.cpp +++ b/src/tracing/esp32_trace/counter.cpp @@ -16,16 +16,18 @@ * limitations under the License. */ +#include #include +using namespace chip; + namespace Insights { -ESPInsightsCounter * ESPInsightsCounter::mHead = nullptr; +ESPInsightsCounter * ESPInsightsCounter::mHead = nullptr; // Pointer to head of the counter linked list ESPInsightsCounter * ESPInsightsCounter::GetInstance(const char * label, const char * group) { - ESPInsightsCounter * current = mHead; - ESPInsightsCounter * previous = nullptr; + ESPInsightsCounter * current = mHead; // Provisional pointer to traverse the counter list while (current != nullptr) { @@ -34,13 +36,17 @@ ESPInsightsCounter * ESPInsightsCounter::GetInstance(const char * label, const c current->instanceCount++; return current; } - previous = current; - current = current->mNext; + current = current->mNext; } - ESPInsightsCounter * newInstance = new ESPInsightsCounter(label, group); + // Allocate a new instance if counter is not present in the list. + void * ptr = Platform::MemoryAlloc(sizeof(ESPInsightsCounter)); + VerifyOrDie(ptr != nullptr); + + ESPInsightsCounter * newInstance = new (ptr) ESPInsightsCounter(label, group); newInstance->mNext = mHead; mHead = newInstance; + return newInstance; } diff --git a/src/tracing/esp32_trace/counter.h b/src/tracing/esp32_trace/counter.h index f306023421fc50..36fe71dae64d0a 100644 --- a/src/tracing/esp32_trace/counter.h +++ b/src/tracing/esp32_trace/counter.h @@ -18,24 +18,26 @@ #include #include -#include +#include +#include + +#define PATH "insights.cnt" -#define PATH1 "sys.cnt" namespace Insights { class ESPInsightsCounter { private: - static ESPInsightsCounter * mHead; - char label[50]; + static ESPInsightsCounter * mHead; // head of the counter list + char label[50]; // unique key char group[50]; int instanceCount; - ESPInsightsCounter * mNext; + ESPInsightsCounter * mNext; // pointer to point to the next entry in the list bool registered = false; ESPInsightsCounter(const char * labelParam, const char * groupParam) : instanceCount(1), mNext(nullptr) { - strncpy(label, labelParam, sizeof(label)); - strncpy(group, groupParam, sizeof(group)); + chip::Platform::CopyString(label, sizeof(label), labelParam); + chip::Platform::CopyString(group, sizeof(group), groupParam); } public: @@ -47,10 +49,12 @@ class ESPInsightsCounter { if (!registered) { - esp_diag_metrics_register("SYS_CNT", label, label, PATH1, ESP_DIAG_DATA_TYPE_UINT); + esp_diag_metrics_register("SYS_CNT" /* Tag of metrics */, label /* Unique key 8 */, + label /* label displayed on dashboard */, PATH /* hierarchical path */, + ESP_DIAG_DATA_TYPE_UINT /* data_type */); registered = true; } - ESP_LOGI("Mtr", "Label = %s Count = %d", label, instanceCount); + ESP_LOGI("mtr", "Label = %s Count = %d", label, instanceCount); esp_diag_metrics_add_uint(label, instanceCount); } }; diff --git a/src/tracing/perfetto/BUILD.gn b/src/tracing/perfetto/BUILD.gn index e2743a4ed9b4d9..64d4cd1d5d947b 100644 --- a/src/tracing/perfetto/BUILD.gn +++ b/src/tracing/perfetto/BUILD.gn @@ -75,6 +75,7 @@ static_library("perfetto") { public_deps = [ ":perfetto_tracing", "${chip_root}/src/lib/address_resolve", + "${chip_root}/src/tracing", "${chip_root}/src/transport", "${chip_root}/third_party/perfetto:sdk", ] diff --git a/src/tracing/perfetto/include/matter/tracing/macros_impl.h b/src/tracing/perfetto/include/matter/tracing/macros_impl.h index 74cfbd20f33304..69fab7b254b6a3 100644 --- a/src/tracing/perfetto/include/matter/tracing/macros_impl.h +++ b/src/tracing/perfetto/include/matter/tracing/macros_impl.h @@ -34,6 +34,6 @@ PERFETTO_DEFINE_CATEGORIES(perfetto::Category("Matter").SetDescription("Matter t #define MATTER_TRACE_COUNTER(label, group) \ do \ { \ - static int count##_label = 0; \ - TRACE_COUNTER("Matter", perfetto::CounterTrack(label), ++count##_label); \ + static int count##_label##_group = 0; \ + TRACE_COUNTER("Matter", perfetto::CounterTrack(label), ++count##_label##_group); \ } while (0) From 21e8cb6753e821eb326d0ed0fe65206ffdfc4052 Mon Sep 17 00:00:00 2001 From: shripad621git Date: Fri, 2 Feb 2024 12:36:46 +0530 Subject: [PATCH 09/11] Removed the group from counter trace macro --- src/protocols/secure_channel/CASESession.cpp | 10 +++---- src/protocols/secure_channel/PASESession.cpp | 8 +++--- src/tracing/backend.h | 2 +- src/tracing/esp32_trace/counter.cpp | 10 +++---- src/tracing/esp32_trace/counter.h | 26 ++++++++++++++----- src/tracing/esp32_trace/esp32_tracing.cpp | 4 +-- src/tracing/esp32_trace/esp32_tracing.h | 2 +- .../include/matter/tracing/macros_impl.h | 2 +- src/tracing/json/json_tracing.cpp | 3 +-- src/tracing/json/json_tracing.h | 2 +- .../include/matter/tracing/macros_impl.h | 2 +- .../include/matter/tracing/macros_impl.h | 6 ++--- src/tracing/registry.cpp | 4 +-- src/tracing/registry.h | 2 +- 14 files changed, 48 insertions(+), 35 deletions(-) diff --git a/src/protocols/secure_channel/CASESession.cpp b/src/protocols/secure_channel/CASESession.cpp index 5c6d7c257896c1..1d6cb27138a496 100644 --- a/src/protocols/secure_channel/CASESession.cpp +++ b/src/protocols/secure_channel/CASESession.cpp @@ -552,7 +552,7 @@ void CASESession::OnResponseTimeout(ExchangeContext * ec) VerifyOrReturn(mExchangeCtxt == ec, ChipLogError(SecureChannel, "CASESession::OnResponseTimeout exchange doesn't match")); ChipLogError(SecureChannel, "CASESession timed out while waiting for a response from the peer. Current state was %u", to_underlying(mState)); - MATTER_TRACE_COUNTER("CASETimeout", "CASESession"); + MATTER_TRACE_COUNTER("CASETimeout"); // Discard the exchange so that Clear() doesn't try aborting it. The // exchange will handle that. DiscardExchange(); @@ -845,7 +845,7 @@ CHIP_ERROR CASESession::HandleSigma1(System::PacketBufferHandle && msg) ByteSpan initiatorRandom; ChipLogProgress(SecureChannel, "Received Sigma1 msg"); - MATTER_TRACE_COUNTER("Sigma1", "CASESession"); + MATTER_TRACE_COUNTER("Sigma1"); bool sessionResumptionRequested = false; ByteSpan resumptionId; @@ -1102,7 +1102,7 @@ CHIP_ERROR CASESession::SendSigma2() mState = State::kSentSigma2; ChipLogProgress(SecureChannel, "Sent Sigma2 msg"); - MATTER_TRACE_COUNTER("Sigma2", "CASESession"); + MATTER_TRACE_COUNTER("Sigma2"); return CHIP_NO_ERROR; } @@ -1119,7 +1119,7 @@ CHIP_ERROR CASESession::HandleSigma2Resume(System::PacketBufferHandle && msg) uint32_t decodeTagIdSeq = 0; ChipLogDetail(SecureChannel, "Received Sigma2Resume msg"); - MATTER_TRACE_COUNTER("Sigma2Resume", "CASESession"); + MATTER_TRACE_COUNTER("Sigma2Resume"); uint8_t sigma2ResumeMIC[CHIP_CRYPTO_AEAD_MIC_LENGTH_BYTES]; @@ -1589,7 +1589,7 @@ CHIP_ERROR CASESession::HandleSigma3a(System::PacketBufferHandle && msg) uint8_t msg_salt[kIPKSize + kSHA256_Hash_Length]; ChipLogProgress(SecureChannel, "Received Sigma3 msg"); - MATTER_TRACE_COUNTER("Sigma3", "CASESession"); + MATTER_TRACE_COUNTER("Sigma3"); auto helper = WorkHelper::Create(*this, &HandleSigma3b, &CASESession::HandleSigma3c); VerifyOrExit(helper, err = CHIP_ERROR_NO_MEMORY); diff --git a/src/protocols/secure_channel/PASESession.cpp b/src/protocols/secure_channel/PASESession.cpp index 7cb8b821c7505f..980a30de79ec1e 100644 --- a/src/protocols/secure_channel/PASESession.cpp +++ b/src/protocols/secure_channel/PASESession.cpp @@ -251,7 +251,7 @@ void PASESession::OnResponseTimeout(ExchangeContext * ec) // If we were waiting for something, mNextExpectedMsg had better have a value. ChipLogError(SecureChannel, "PASESession timed out while waiting for a response from the peer. Expected message type was %u", to_underlying(mNextExpectedMsg.Value())); - MATTER_TRACE_COUNTER("PASETimeout", "PASESession"); + MATTER_TRACE_COUNTER("PASETimeout"); // Discard the exchange so that Clear() doesn't try closing it. The // exchange will handle that. DiscardExchange(); @@ -622,7 +622,7 @@ CHIP_ERROR PASESession::HandleMsg1_and_SendMsg2(System::PacketBufferHandle && ms } ChipLogDetail(SecureChannel, "Sent spake2p msg2"); - MATTER_TRACE_COUNTER("Pake2", "PASESession"); + MATTER_TRACE_COUNTER("Pake2"); exit: @@ -714,7 +714,7 @@ CHIP_ERROR PASESession::HandleMsg3(System::PacketBufferHandle && msg) CHIP_ERROR err = CHIP_NO_ERROR; ChipLogDetail(SecureChannel, "Received spake2p msg3"); - MATTER_TRACE_COUNTER("Pake3", "PASESession"); + MATTER_TRACE_COUNTER("Pake3"); mNextExpectedMsg.ClearValue(); @@ -875,7 +875,7 @@ CHIP_ERROR PASESession::OnMessageReceived(ExchangeContext * exchange, const Payl DiscardExchange(); Clear(); ChipLogError(SecureChannel, "Failed during PASE session setup: %" CHIP_ERROR_FORMAT, err.Format()); - MATTER_TRACE_COUNTER("PASEFail", "PASESession"); + MATTER_TRACE_COUNTER("PASEFail"); // Do this last in case the delegate frees us. NotifySessionEstablishmentError(err); } diff --git a/src/tracing/backend.h b/src/tracing/backend.h index 36fda9ae33eea9..3d8bec8ada501f 100644 --- a/src/tracing/backend.h +++ b/src/tracing/backend.h @@ -63,7 +63,7 @@ class Backend : public ::chip::IntrusiveListNodeBase<> /// Trace a zero-sized event virtual void TraceInstant(const char * label, const char * group) {} - virtual void TraceCounter(const char * label, const char * group) {} + virtual void TraceCounter(const char * label) {} virtual void LogMessageSend(MessageSendInfo &) { TraceInstant("MessageSent", "Messaging"); } virtual void LogMessageReceived(MessageReceivedInfo &) { TraceInstant("MessageReceived", "Messaging"); } diff --git a/src/tracing/esp32_trace/counter.cpp b/src/tracing/esp32_trace/counter.cpp index 65826c656057ae..251ca50bd4eb59 100644 --- a/src/tracing/esp32_trace/counter.cpp +++ b/src/tracing/esp32_trace/counter.cpp @@ -22,16 +22,16 @@ using namespace chip; namespace Insights { -ESPInsightsCounter * ESPInsightsCounter::mHead = nullptr; // Pointer to head of the counter linked list +// It need not be freed. One time allocation . Need to track counters till the device being online. +ESPInsightsCounter * ESPInsightsCounter::mHead = nullptr; -ESPInsightsCounter * ESPInsightsCounter::GetInstance(const char * label, const char * group) +ESPInsightsCounter * ESPInsightsCounter::GetInstance(const char * label) { - ESPInsightsCounter * current = mHead; // Provisional pointer to traverse the counter list while (current != nullptr) { - if (strcmp(current->label, label) == 0 && strcmp(current->group, group) == 0) + if (strcmp(current->label, label) == 0) { current->instanceCount++; return current; @@ -43,7 +43,7 @@ ESPInsightsCounter * ESPInsightsCounter::GetInstance(const char * label, const c void * ptr = Platform::MemoryAlloc(sizeof(ESPInsightsCounter)); VerifyOrDie(ptr != nullptr); - ESPInsightsCounter * newInstance = new (ptr) ESPInsightsCounter(label, group); + ESPInsightsCounter * newInstance = new (ptr) ESPInsightsCounter(label); newInstance->mNext = mHead; mHead = newInstance; diff --git a/src/tracing/esp32_trace/counter.h b/src/tracing/esp32_trace/counter.h index 36fe71dae64d0a..7db83fe0afa790 100644 --- a/src/tracing/esp32_trace/counter.h +++ b/src/tracing/esp32_trace/counter.h @@ -20,6 +20,7 @@ #include #include #include +#include #define PATH "insights.cnt" @@ -28,20 +29,18 @@ class ESPInsightsCounter { private: static ESPInsightsCounter * mHead; // head of the counter list - char label[50]; // unique key - char group[50]; + const char * label; // unique key int instanceCount; ESPInsightsCounter * mNext; // pointer to point to the next entry in the list bool registered = false; - ESPInsightsCounter(const char * labelParam, const char * groupParam) : instanceCount(1), mNext(nullptr) + ESPInsightsCounter(const char * labelParam) : instanceCount(1), mNext(nullptr) { - chip::Platform::CopyString(label, sizeof(label), labelParam); - chip::Platform::CopyString(group, sizeof(group), groupParam); + label = chip::Platform::MemoryAllocString(labelParam, strlen(labelParam)); } public: - static ESPInsightsCounter * GetInstance(const char * label, const char * group); + static ESPInsightsCounter * GetInstance(const char * label); int GetInstanceCount() const; @@ -57,6 +56,21 @@ class ESPInsightsCounter ESP_LOGI("mtr", "Label = %s Count = %d", label, instanceCount); esp_diag_metrics_add_uint(label, instanceCount); } + + // Destructor to free the dynamically allocated memory + ~ESPInsightsCounter() + { + chip::Platform::MemoryFree((void *) label); + + ESPInsightsCounter * current = mHead; + while (current) + { + ESPInsightsCounter * next = current->mNext; + chip::Platform::MemoryFree((void *) current); + current = next; + } + mHead = nullptr; + } }; } // namespace Insights diff --git a/src/tracing/esp32_trace/esp32_tracing.cpp b/src/tracing/esp32_trace/esp32_tracing.cpp index 3bcd6ec5842479..7937772606eec0 100644 --- a/src/tracing/esp32_trace/esp32_tracing.cpp +++ b/src/tracing/esp32_trace/esp32_tracing.cpp @@ -149,9 +149,9 @@ void ESP32Backend::LogNodeDiscovered(NodeDiscoveredInfo & info) {} void ESP32Backend::LogNodeDiscoveryFailed(NodeDiscoveryFailedInfo & info) {} -void ESP32Backend::TraceCounter(const char * label, const char * group) +void ESP32Backend::TraceCounter(const char * label) { - ::Insights::ESPInsightsCounter::GetInstance(label, group)->ReportMetrics(); + ::Insights::ESPInsightsCounter::GetInstance(label)->ReportMetrics(); } void ESP32Backend::TraceBegin(const char * label, const char * group) { diff --git a/src/tracing/esp32_trace/esp32_tracing.h b/src/tracing/esp32_trace/esp32_tracing.h index fb4125518281fa..6d9a277e06561d 100644 --- a/src/tracing/esp32_trace/esp32_tracing.h +++ b/src/tracing/esp32_trace/esp32_tracing.h @@ -28,7 +28,7 @@ class ESP32Backend : public ::chip::Tracing::Backend /// Trace a zero-sized event void TraceInstant(const char * label, const char * group) override; - void TraceCounter(const char * label, const char * group) override; + void TraceCounter(const char * label) override; void LogMessageSend(MessageSendInfo &) override; void LogMessageReceived(MessageReceivedInfo &) override; diff --git a/src/tracing/esp32_trace/include/matter/tracing/macros_impl.h b/src/tracing/esp32_trace/include/matter/tracing/macros_impl.h index c439922f2cd5ff..b95c20754237a1 100644 --- a/src/tracing/esp32_trace/include/matter/tracing/macros_impl.h +++ b/src/tracing/esp32_trace/include/matter/tracing/macros_impl.h @@ -28,7 +28,7 @@ #define MATTER_TRACE_BEGIN(label, group) ::chip::Tracing::Internal::Begin(label, group) #define MATTER_TRACE_END(label, group) ::chip::Tracing::Internal::End(label, group) #define MATTER_TRACE_INSTANT(label, group) ::chip::Tracing::Internal::Instant(label, group) -#define MATTER_TRACE_COUNTER(label, group) ::chip::Tracing::Internal::Counter(label, group) +#define MATTER_TRACE_COUNTER(label) ::chip::Tracing::Internal::Counter(label) namespace chip { namespace Tracing { diff --git a/src/tracing/json/json_tracing.cpp b/src/tracing/json/json_tracing.cpp index 07b929670ee073..8dad5635b6585d 100644 --- a/src/tracing/json/json_tracing.cpp +++ b/src/tracing/json/json_tracing.cpp @@ -275,7 +275,7 @@ void JsonBackend::TraceInstant(const char * label, const char * group) OutputValue(value); } -void JsonBackend::TraceCounter(const char * label, const char * group) +void JsonBackend::TraceCounter(const char * label) { std::string counterId = std::string(label); if (mCounters.find(counterId) == mCounters.end()) @@ -289,7 +289,6 @@ void JsonBackend::TraceCounter(const char * label, const char * group) ::Json::Value value; value["event"] = "TraceCounter"; value["label"] = label; - value["group"] = group; value["count"] = mCounters[counterId]; // Output the counter event diff --git a/src/tracing/json/json_tracing.h b/src/tracing/json/json_tracing.h index 9fd171036cf6d1..302c4bd5b5a485 100644 --- a/src/tracing/json/json_tracing.h +++ b/src/tracing/json/json_tracing.h @@ -51,7 +51,7 @@ class JsonBackend : public ::chip::Tracing::Backend void TraceBegin(const char * label, const char * group) override; void TraceEnd(const char * label, const char * group) override; void TraceInstant(const char * label, const char * group) override; - void TraceCounter(const char * label, const char * group) override; + void TraceCounter(const char * label) override; void LogMessageSend(MessageSendInfo &) override; void LogMessageReceived(MessageReceivedInfo &) override; void LogNodeLookup(NodeLookupInfo &) override; diff --git a/src/tracing/multiplexed/include/matter/tracing/macros_impl.h b/src/tracing/multiplexed/include/matter/tracing/macros_impl.h index 90329e477b9ec2..8b3e728289dc1d 100644 --- a/src/tracing/multiplexed/include/matter/tracing/macros_impl.h +++ b/src/tracing/multiplexed/include/matter/tracing/macros_impl.h @@ -28,7 +28,7 @@ #define MATTER_TRACE_BEGIN(label, group) ::chip::Tracing::Internal::Begin(label, group) #define MATTER_TRACE_END(label, group) ::chip::Tracing::Internal::End(label, group) #define MATTER_TRACE_INSTANT(label, group) ::chip::Tracing::Internal::Instant(label, group) -#define MATTER_TRACE_COUNTER(label, group) ::chip::Tracing::Internal::Counter(label, group) +#define MATTER_TRACE_COUNTER(label) ::chip::Tracing::Internal::Counter(label) namespace chip { namespace Tracing { diff --git a/src/tracing/perfetto/include/matter/tracing/macros_impl.h b/src/tracing/perfetto/include/matter/tracing/macros_impl.h index 69fab7b254b6a3..9b05d2f3539a45 100644 --- a/src/tracing/perfetto/include/matter/tracing/macros_impl.h +++ b/src/tracing/perfetto/include/matter/tracing/macros_impl.h @@ -31,9 +31,9 @@ PERFETTO_DEFINE_CATEGORIES(perfetto::Category("Matter").SetDescription("Matter t #define MATTER_TRACE_INSTANT(label, group) TRACE_EVENT_INSTANT("Matter", label, "class_name", group) #define MATTER_TRACE_SCOPE(label, group) TRACE_EVENT("Matter", label, "class_name", group) -#define MATTER_TRACE_COUNTER(label, group) \ +#define MATTER_TRACE_COUNTER(label) \ do \ { \ - static int count##_label##_group = 0; \ - TRACE_COUNTER("Matter", perfetto::CounterTrack(label), ++count##_label##_group); \ + static int count##_label = 0; \ + TRACE_COUNTER("Matter", label, ++count##_label); \ } while (0) diff --git a/src/tracing/registry.cpp b/src/tracing/registry.cpp index 80f2a7545a3f21..7c43442b5b83db 100644 --- a/src/tracing/registry.cpp +++ b/src/tracing/registry.cpp @@ -76,11 +76,11 @@ void Instant(const char * label, const char * group) } } -void Counter(const char * label, const char * group) +void Counter(const char * label) { for (auto & backend : gTracingBackends) { - backend.TraceCounter(label, group); + backend.TraceCounter(label); } } diff --git a/src/tracing/registry.h b/src/tracing/registry.h index 1dce9c13149f6d..083ebcb2225ad2 100644 --- a/src/tracing/registry.h +++ b/src/tracing/registry.h @@ -76,7 +76,7 @@ namespace Internal { void Begin(const char * label, const char * group); void End(const char * label, const char * group); void Instant(const char * label, const char * group); -void Counter(const char * label, const char * group); +void Counter(const char * label); void LogMessageSend(::chip::Tracing::MessageSendInfo & info); void LogMessageReceived(::chip::Tracing::MessageReceivedInfo & info); From 4fc8ddbeb374cf434bd7391ebb268c22ec15d9f8 Mon Sep 17 00:00:00 2001 From: shripad621git Date: Fri, 2 Feb 2024 15:41:03 +0530 Subject: [PATCH 10/11] Refactored some code --- .../wifi-network-diagnostics-server.cpp | 1 + src/protocols/secure_channel/CASEServer.cpp | 1 + src/tracing/esp32_trace/counter.cpp | 35 +++++++++++++++++-- src/tracing/esp32_trace/counter.h | 30 ++-------------- src/tracing/json/json_tracing.cpp | 2 +- src/tracing/perfetto/perfetto_tracing.h | 1 - 6 files changed, 38 insertions(+), 32 deletions(-) diff --git a/src/app/clusters/wifi-network-diagnostics-server/wifi-network-diagnostics-server.cpp b/src/app/clusters/wifi-network-diagnostics-server/wifi-network-diagnostics-server.cpp index d94059ed8299a8..bd02f36d731cf2 100644 --- a/src/app/clusters/wifi-network-diagnostics-server/wifi-network-diagnostics-server.cpp +++ b/src/app/clusters/wifi-network-diagnostics-server/wifi-network-diagnostics-server.cpp @@ -245,6 +245,7 @@ class WiFiDiagnosticsDelegate : public DeviceLayer::WiFiDiagnosticsDelegate { MATTER_TRACE_SCOPE("OnDisconnectionDetected", "WiFiDiagnosticsDelegate"); ChipLogProgress(Zcl, "WiFiDiagnosticsDelegate: OnDisconnectionDetected"); + for (auto endpoint : EnabledEndpointsWithServerCluster(WiFiNetworkDiagnostics::Id)) { // If WiFi Network Diagnostics cluster is implemented on this endpoint diff --git a/src/protocols/secure_channel/CASEServer.cpp b/src/protocols/secure_channel/CASEServer.cpp index 47a92621f5078e..2ad196a31b9461 100644 --- a/src/protocols/secure_channel/CASEServer.cpp +++ b/src/protocols/secure_channel/CASEServer.cpp @@ -182,6 +182,7 @@ void CASEServer::OnSessionEstablishmentError(CHIP_ERROR err) { MATTER_TRACE_SCOPE("OnSessionEstablishmentError", "CASEServer"); ChipLogError(Inet, "CASE Session establishment failed: %" CHIP_ERROR_FORMAT, err.Format()); + MATTER_TRACE_SCOPE("CASEFail", "CASESession"); PrepareForSessionEstablishment(); } diff --git a/src/tracing/esp32_trace/counter.cpp b/src/tracing/esp32_trace/counter.cpp index 251ca50bd4eb59..c2a56726b98958 100644 --- a/src/tracing/esp32_trace/counter.cpp +++ b/src/tracing/esp32_trace/counter.cpp @@ -1,6 +1,6 @@ /* * - * Copyright (c) 2023 Project CHIP Authors + * Copyright (c) 2024 Project CHIP Authors * All rights reserved. * * Licensed under the Apache License, Version 2.0 (the "License"); @@ -22,12 +22,13 @@ using namespace chip; namespace Insights { -// It need not be freed. One time allocation . Need to track counters till the device being online. + +// This is a one time allocation for counters. ESPInsightsCounter * ESPInsightsCounter::mHead = nullptr; ESPInsightsCounter * ESPInsightsCounter::GetInstance(const char * label) { - ESPInsightsCounter * current = mHead; // Provisional pointer to traverse the counter list + ESPInsightsCounter * current = mHead; while (current != nullptr) { @@ -55,4 +56,32 @@ int ESPInsightsCounter::GetInstanceCount() const return instanceCount; } +void ESPInsightsCounter::ReportMetrics() +{ + if (!registered) + { + esp_diag_metrics_register("SYS_CNT" /* Tag of metrics */, label /* Unique key 8 */, + label /* label displayed on dashboard */, PATH /* hierarchical path */, + ESP_DIAG_DATA_TYPE_UINT /* data_type */); + registered = true; + } + ESP_LOGI("mtr", "Label = %s Count = %d", label, instanceCount); + esp_diag_metrics_add_uint(label, instanceCount); +} + +// Destructor to free the dynamically allocated memory +ESPInsightsCounter::~ESPInsightsCounter() +{ + chip::Platform::MemoryFree((void *) label); + + ESPInsightsCounter * current = mHead; + while (current) + { + ESPInsightsCounter * next = current->mNext; + chip::Platform::MemoryFree((void *) current); + current = next; + } + mHead = nullptr; +} + } // namespace Insights diff --git a/src/tracing/esp32_trace/counter.h b/src/tracing/esp32_trace/counter.h index 7db83fe0afa790..7facd74190a0ee 100644 --- a/src/tracing/esp32_trace/counter.h +++ b/src/tracing/esp32_trace/counter.h @@ -1,6 +1,6 @@ /* * - * Copyright (c) 2023 Project CHIP Authors + * Copyright (c) 2024 Project CHIP Authors * All rights reserved. * * Licensed under the Apache License, Version 2.0 (the "License"); @@ -44,33 +44,9 @@ class ESPInsightsCounter int GetInstanceCount() const; - void ReportMetrics() - { - if (!registered) - { - esp_diag_metrics_register("SYS_CNT" /* Tag of metrics */, label /* Unique key 8 */, - label /* label displayed on dashboard */, PATH /* hierarchical path */, - ESP_DIAG_DATA_TYPE_UINT /* data_type */); - registered = true; - } - ESP_LOGI("mtr", "Label = %s Count = %d", label, instanceCount); - esp_diag_metrics_add_uint(label, instanceCount); - } + void ReportMetrics(); - // Destructor to free the dynamically allocated memory - ~ESPInsightsCounter() - { - chip::Platform::MemoryFree((void *) label); - - ESPInsightsCounter * current = mHead; - while (current) - { - ESPInsightsCounter * next = current->mNext; - chip::Platform::MemoryFree((void *) current); - current = next; - } - mHead = nullptr; - } + ~ESPInsightsCounter(); }; } // namespace Insights diff --git a/src/tracing/json/json_tracing.cpp b/src/tracing/json/json_tracing.cpp index 8dad5635b6585d..af66e12a61a50b 100644 --- a/src/tracing/json/json_tracing.cpp +++ b/src/tracing/json/json_tracing.cpp @@ -23,10 +23,10 @@ #include #include #include +#include #include #include -#include #include diff --git a/src/tracing/perfetto/perfetto_tracing.h b/src/tracing/perfetto/perfetto_tracing.h index a7858c78305c67..901165e35a548d 100644 --- a/src/tracing/perfetto/perfetto_tracing.h +++ b/src/tracing/perfetto/perfetto_tracing.h @@ -17,7 +17,6 @@ */ #pragma once -#include #include #include From 5b0e3fc87a66d3b4e342b6533ab9bc4f98ee6efa Mon Sep 17 00:00:00 2001 From: shripad621git Date: Fri, 2 Feb 2024 20:43:44 +0530 Subject: [PATCH 11/11] Addressed the string and class documentation related review comments --- src/tracing/esp32_trace/counter.cpp | 19 ++----------------- src/tracing/esp32_trace/counter.h | 19 +++++++++++-------- 2 files changed, 13 insertions(+), 25 deletions(-) diff --git a/src/tracing/esp32_trace/counter.cpp b/src/tracing/esp32_trace/counter.cpp index c2a56726b98958..0b315a87007484 100644 --- a/src/tracing/esp32_trace/counter.cpp +++ b/src/tracing/esp32_trace/counter.cpp @@ -23,7 +23,7 @@ using namespace chip; namespace Insights { -// This is a one time allocation for counters. +// This is a one time allocation for counters. It is not supposed to be freed. ESPInsightsCounter * ESPInsightsCounter::mHead = nullptr; ESPInsightsCounter * ESPInsightsCounter::GetInstance(const char * label) @@ -61,7 +61,7 @@ void ESPInsightsCounter::ReportMetrics() if (!registered) { esp_diag_metrics_register("SYS_CNT" /* Tag of metrics */, label /* Unique key 8 */, - label /* label displayed on dashboard */, PATH /* hierarchical path */, + label /* label displayed on dashboard */, "insights.cnt" /* hierarchical path */, ESP_DIAG_DATA_TYPE_UINT /* data_type */); registered = true; } @@ -69,19 +69,4 @@ void ESPInsightsCounter::ReportMetrics() esp_diag_metrics_add_uint(label, instanceCount); } -// Destructor to free the dynamically allocated memory -ESPInsightsCounter::~ESPInsightsCounter() -{ - chip::Platform::MemoryFree((void *) label); - - ESPInsightsCounter * current = mHead; - while (current) - { - ESPInsightsCounter * next = current->mNext; - chip::Platform::MemoryFree((void *) current); - current = next; - } - mHead = nullptr; -} - } // namespace Insights diff --git a/src/tracing/esp32_trace/counter.h b/src/tracing/esp32_trace/counter.h index 7facd74190a0ee..bfa6205c341a13 100644 --- a/src/tracing/esp32_trace/counter.h +++ b/src/tracing/esp32_trace/counter.h @@ -22,9 +22,17 @@ #include #include -#define PATH "insights.cnt" - namespace Insights { + +/* + * + * This class is used to monotonically increment the counters as per the label of the counter macro + * 'MATTER_TRACE_COUNTER(label)' and report the metrics to esp-insights. + * As per the label of the counter macro, it adds the counter in the linked list with the name label if not + * present and returns the same instance and increments the value if the counter is already present + * in the list. + */ + class ESPInsightsCounter { private: @@ -34,10 +42,7 @@ class ESPInsightsCounter ESPInsightsCounter * mNext; // pointer to point to the next entry in the list bool registered = false; - ESPInsightsCounter(const char * labelParam) : instanceCount(1), mNext(nullptr) - { - label = chip::Platform::MemoryAllocString(labelParam, strlen(labelParam)); - } + ESPInsightsCounter(const char * labelParam) : label(labelParam), instanceCount(1), mNext(nullptr) {} public: static ESPInsightsCounter * GetInstance(const char * label); @@ -45,8 +50,6 @@ class ESPInsightsCounter int GetInstanceCount() const; void ReportMetrics(); - - ~ESPInsightsCounter(); }; } // namespace Insights