[GitHub] [incubator-pegasus] acelyc111 commented on a diff in pull request #1117: feat: introduce data sink to collect snapshot from each metric periodically

[GitBox <gi...@apache.org>]

acelyc111 commented on code in PR #1117:
URL: https://github.com/apache/incubator-pegasus/pull/1117#discussion_r949857330


##########
src/rdsn/include/dsn/utility/metrics.h:
##########
@@ -134,10 +135,13 @@ class metric_prototype;
 class metric;
 using metric_ptr = ref_ptr<metric>;
 
+class metric_data_sink;
+using metric_data_sink_ptr = ref_ptr<metric_data_sink>;
+
 class metric_entity : public ref_counter
 {
 public:
-    using attr_map = std::unordered_map<std::string, std::string>;
+    using attr_map = std::map<std::string, std::string>;

Review Comment:
   why change to use std:map?



##########
src/rdsn/include/dsn/utility/metrics.h:
##########
@@ -215,6 +221,54 @@ class metric_entity_prototype
     DISALLOW_COPY_AND_ASSIGN(metric_entity_prototype);
 };
 
+// `metric_timer` is a timer class that runs metric-related computations periodically, such as

Review Comment:
   > runs metric-related computations periodically
   
   Even though there isn't any thirdparty monitoring system, it will still caculate periodically? Would it be a bit of cost?
   Another way is to calculate lazily, that is to say, calculate will only be triggered when any body request these metrics, you can define some callbacks for that.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: dev-unsubscribe@pegasus.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@pegasus.apache.org
For additional commands, e-mail: dev-help@pegasus.apache.org