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


##########
src/utils/metrics.h:
##########
@@ -331,6 +353,17 @@ class metric_prototype_with : public metric_prototype
     DISALLOW_COPY_AND_ASSIGN(metric_prototype_with);
 };
 
+const std::string kMetricTypeField = "type";
+const std::string kMetricNameField = "name";
+const std::string kMetricUnitField = "unit";
+const std::string kMetricDescField = "desc";
+const metric_fields_type kMetricPrototypeFields = {

Review Comment:
   Will kMetricPrototypeFields be used anywhere else except 
get_all_single_value_metric_fields(), if not, make it there as a local variable 
and do not expose it in header.



##########
src/utils/metrics.h:
##########
@@ -131,6 +132,26 @@
 
 namespace dsn {
 
+using metric_fields_type = std::unordered_set<std::string>;
+
+// This struct includes a set of filters for both entities and metrics 
requested by client.
+struct metric_filters
+{
+    // According to the parameters requested by client, this function will 
filter metric
+    // fields that will be put in the response. `with_metric_fields` includes 
all the metric
+    // fields that are wanted by client.
+    bool has_metric_field(const std::string &field_name) const
+    {
+        if (with_metric_fields.empty()) {

Review Comment:
   Is it necessary? In most cases `with_metric_fields` is not empty, then there 
will be 2 judgements, IMO, leave line 149 is enough.



-- 
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: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]


---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to