From f83409276160be0cfc679121fcd8e94f02676118 Mon Sep 17 00:00:00 2001 From: shripad621git <79364691+shripad621git@users.noreply.github.com> Date: Thu, 7 Mar 2024 10:47:55 +0530 Subject: [PATCH] [ESP32] Tracing : Added the fix of data type mismatch while registering the metrics in esp32 tracing framework. (#32355) * Fixed the type mismatch in the esp32 tracing framework * Removed the heap metrics as insights framework maps traces to heap metrics * addressed review comments * restlyed * Added a check to ensure one type associated with one key * Addressed the further review comments --- scripts/tools/check_includes_config.py | 3 ++ src/tracing/esp32_trace/esp32_tracing.cpp | 56 +++++++++++++++++++---- src/tracing/esp32_trace/esp32_tracing.h | 6 ++- 3 files changed, 54 insertions(+), 11 deletions(-) diff --git a/scripts/tools/check_includes_config.py b/scripts/tools/check_includes_config.py index f4715888832bf3..26689e4a2f85b4 100644 --- a/scripts/tools/check_includes_config.py +++ b/scripts/tools/check_includes_config.py @@ -160,6 +160,9 @@ 'src/tracing/json/json_tracing.cpp': {'string', 'sstream'}, 'src/tracing/json/json_tracing.h': {'fstream', 'unordered_map'}, + # esp32 tracing + 'src/tracing/esp32_trace/esp32_tracing.h': {'unordered_map'}, + # Not intended for embedded clients 'src/app/PendingResponseTrackerImpl.h': {'unordered_set'}, diff --git a/src/tracing/esp32_trace/esp32_tracing.cpp b/src/tracing/esp32_trace/esp32_tracing.cpp index db5f09ee9a751c..7b338466378945 100644 --- a/src/tracing/esp32_trace/esp32_tracing.cpp +++ b/src/tracing/esp32_trace/esp32_tracing.cpp @@ -17,6 +17,7 @@ */ #include +#include #include #include #include @@ -134,10 +135,7 @@ void RemoveHashFromPermitlist(const char * str) #define LOG_HEAP_INFO(label, group, entry_exit) \ do \ { \ - ESP_DIAG_EVENT("MTR_TRC", "%s - %s - %s Min Free heap - %u - LFB - %u Start free heap - %u", entry_exit, label, group, \ - heap_caps_get_minimum_free_size(MALLOC_CAP_8BIT), \ - heap_caps_get_largest_free_block(MALLOC_CAP_INTERNAL | MALLOC_CAP_8BIT), \ - heap_caps_get_free_size(MALLOC_CAP_8BIT)); \ + ESP_DIAG_EVENT("MTR_TRC", "%s - %s - %s", entry_exit, label, group); \ } while (0) void ESP32Backend::LogMessageReceived(MessageReceivedInfo & info) {} @@ -155,34 +153,71 @@ void ESP32Backend::TraceCounter(const char * label) ::Insights::ESPInsightsCounter::GetInstance(label)->ReportMetrics(); } +void ESP32Backend::RegisterMetric(const char * key, ValueType type) +{ + // Check for the same key will not have two different types. + if (mRegisteredMetrics.find(key) != mRegisteredMetrics.end()) + { + if (mRegisteredMetrics[key] != type) + { + ESP_LOGE("SYS.MTR", "Type mismatch for metric key %s", key); + return; + } + } + + switch (type) + { + case ValueType::kUInt32: + esp_diag_metrics_register("SYS_MTR" /*Tag of metrics */, key /* Unique key 8 */, key /* label displayed on dashboard */, + "insights.mtr" /* hierarchical path */, ESP_DIAG_DATA_TYPE_UINT /* data_type */); + break; + + case ValueType::kInt32: + esp_diag_metrics_register("SYS_MTR" /*Tag of metrics */, key /* Unique key 8 */, key /* label displayed on dashboard */, + "insights.mtr" /* hierarchical path */, ESP_DIAG_DATA_TYPE_INT /* data_type */); + break; + + case ValueType::kChipErrorCode: + esp_diag_metrics_register("SYS_MTR" /*Tag of metrics */, key /* Unique key 8 */, key /* label displayed on dashboard */, + "insights.mtr" /* hierarchical path */, ESP_DIAG_DATA_TYPE_UINT /* data_type */); + break; + + case ValueType::kUndefined: + ESP_LOGE("mtr", "failed to register %s as its value is undefined", key); + break; + } + + mRegisteredMetrics[key] = type; +} + void ESP32Backend::LogMetricEvent(const MetricEvent & event) { - if (!mRegistered) + if (mRegisteredMetrics.find(event.key()) == mRegisteredMetrics.end()) { - esp_diag_metrics_register("SYS_MTR" /*Tag of metrics */, event.key() /* Unique key 8 */, - event.key() /* label displayed on dashboard */, "insights.mtr" /* hierarchical path */, - ESP_DIAG_DATA_TYPE_INT /* data_type */); - mRegistered = true; + RegisterMetric(event.key(), event.ValueType()); } - using ValueType = MetricEvent::Value::Type; switch (event.ValueType()) { case ValueType::kInt32: ESP_LOGI("mtr", "The value of %s is %ld ", event.key(), event.ValueInt32()); esp_diag_metrics_add_int(event.key(), event.ValueInt32()); break; + case ValueType::kUInt32: ESP_LOGI("mtr", "The value of %s is %lu ", event.key(), event.ValueUInt32()); esp_diag_metrics_add_uint(event.key(), event.ValueUInt32()); break; + case ValueType::kChipErrorCode: ESP_LOGI("mtr", "The value of %s is error with code %lu ", event.key(), event.ValueErrorCode()); esp_diag_metrics_add_uint(event.key(), event.ValueErrorCode()); break; + case ValueType::kUndefined: ESP_LOGI("mtr", "The value of %s is undefined", event.key()); break; + default: ESP_LOGI("mtr", "The value of %s is of an UNKNOWN TYPE", event.key()); break; @@ -211,6 +246,7 @@ void ESP32Backend::TraceInstant(const char * label, const char * group) { ESP_DIAG_EVENT("MTR_TRC", "Instant : %s -%s", label, group); } + } // namespace Insights } // namespace Tracing } // namespace chip diff --git a/src/tracing/esp32_trace/esp32_tracing.h b/src/tracing/esp32_trace/esp32_tracing.h index 2e08e69d616158..c0903ac72fd8dc 100644 --- a/src/tracing/esp32_trace/esp32_tracing.h +++ b/src/tracing/esp32_trace/esp32_tracing.h @@ -1,5 +1,7 @@ #include #include +#include +#include #include namespace chip { @@ -39,7 +41,9 @@ class ESP32Backend : public ::chip::Tracing::Backend void LogMetricEvent(const MetricEvent &) override; private: - bool mRegistered = false; + using ValueType = MetricEvent::Value::Type; + std::unordered_map mRegisteredMetrics; + void RegisterMetric(const char * key, ValueType type); }; } // namespace Insights