acelyc111 commented on code in PR #1258:
URL:
https://github.com/apache/incubator-pegasus/pull/1258#discussion_r1037018666
##########
src/utils/metrics.h:
##########
@@ -209,8 +201,19 @@ class metric_entity : public ref_counter
};
void close(close_option option);
- void set_attributes(attr_map &&attrs);
+ void set_attributes(const attr_map &attrs);
+
+ void encode_type(metric_json_writer &writer) const;
+
+ void encode_id(metric_json_writer &writer) const;
+
+ static void encode_attrs(metric_json_writer &writer, const attr_map
&attrs);
+ static void encode_metrics(metric_json_writer &writer,
Review Comment:
Same.
##########
src/utils/metrics.h:
##########
@@ -222,6 +225,95 @@ class metric_entity : public ref_counter
using metric_entity_ptr = ref_ptr<metric_entity>;
+// This struct includes a set of filters for both entities and metrics
requested by client.
+struct metric_filters
+{
+ using metric_fields_type = std::unordered_set<std::string>;
+ using entity_types_type = std::vector<std::string>;
+ using entity_ids_type = std::unordered_set<std::string>;
+ using entity_attrs_type = std::vector<std::string>;
+ using entity_metrics_type = std::vector<std::string>;
+
+// NOTICE: empty `white_list` means every field is required by client.
+#define RETURN_MATCHED_WITH_EMPTY_WHITE_LIST(white_list)
\
+ do {
\
+ if (white_list.empty()) {
\
+ return true;
\
+ }
\
+ } while (0)
+
+#define DEFINE_SIMPLE_MATCHER(name)
\
+ template <typename T>
\
+ inline bool match_##name(const T &candidate) const
\
+ {
\
+ return match(candidate, name##s);
\
+ }
+
+ static inline bool match(const char *candidate, const
std::vector<std::string> &white_list)
+ {
+ RETURN_MATCHED_WITH_EMPTY_WHITE_LIST(white_list);
+ // Will use `bool operator==(const string &lhs, const char *rhs);` to
compare each element
+ // in `white_list` with `candidate`.
+ return std::find(white_list.begin(), white_list.end(), candidate) !=
white_list.end();
+ }
+
+ static inline bool match(const std::string &candidate,
+ const std::unordered_set<std::string> &white_list)
+ {
+ RETURN_MATCHED_WITH_EMPTY_WHITE_LIST(white_list);
+ return white_list.find(candidate) != white_list.end();
+ }
+
+ // According to the parameters requested by client, this function will
filter metric
+ // fields that will be put in the response.
+ DEFINE_SIMPLE_MATCHER(with_metric_field)
+
+ DEFINE_SIMPLE_MATCHER(entity_type)
+
+ DEFINE_SIMPLE_MATCHER(entity_id)
+
+ bool match_entity_attrs(const metric_entity::attr_map &candidates) const
+ {
+ RETURN_MATCHED_WITH_EMPTY_WHITE_LIST(entity_attrs);
+
+ // The size of container must be divisible by 2, since attribute name
always pairs
+ // with value in it.
+ CHECK_EQ(entity_attrs.size() & 1, 0);
+
+ for (entity_attrs_type::size_type i = 0; i < entity_attrs.size(); i +=
2) {
+ metric_entity::attr_map::const_iterator iter =
candidates.find(entity_attrs[i]);
Review Comment:
```suggestion
const auto &iter = candidates.find(entity_attrs[i]);
```
##########
src/utils/metrics.h:
##########
@@ -209,8 +201,19 @@ class metric_entity : public ref_counter
};
void close(close_option option);
- void set_attributes(attr_map &&attrs);
+ void set_attributes(const attr_map &attrs);
+
+ void encode_type(metric_json_writer &writer) const;
+
+ void encode_id(metric_json_writer &writer) const;
+
+ static void encode_attrs(metric_json_writer &writer, const attr_map
&attrs);
Review Comment:
Instead of using private static function, would it better to hide it in cpp
as a anonymous namespace function?
##########
src/utils/metrics.cpp:
##########
@@ -24,8 +24,10 @@
namespace dsn {
-metric_entity::metric_entity(const std::string &id, attr_map &&attrs)
- : _id(id), _lock(), _attrs(std::move(attrs)), _metrics()
+metric_entity::metric_entity(const metric_entity_prototype *prototype,
+ const std::string &id,
+ const attr_map &attrs)
+ : _prototype(prototype), _id(id), _lock(), _attrs(attrs), _metrics()
Review Comment:
How about let _lock and _metrics construct implicitly and automatically?
##########
src/utils/metrics.cpp:
##########
@@ -81,19 +83,115 @@ metric_entity::metric_map metric_entity::metrics() const
return _metrics;
}
-void metric_entity::set_attributes(attr_map &&attrs)
+void metric_entity::set_attributes(const attr_map &attrs)
{
utils::auto_write_lock l(_lock);
- _attrs = std::move(attrs);
+ _attrs = attrs;
}
-metric_entity_ptr metric_entity_prototype::instantiate(const std::string &id,
- metric_entity::attr_map
attrs) const
+void metric_entity::encode_type(metric_json_writer &writer) const
+{
+ writer.Key(kMetricEntityTypeField.c_str());
+ json::json_encode(writer, _prototype->name());
+}
+
+void metric_entity::encode_id(metric_json_writer &writer) const
+{
+ writer.Key(kMetricEntityIdField.c_str());
+ json::json_encode(writer, _id);
+}
+
+/*static*/ void metric_entity::encode_attrs(metric_json_writer &writer, const
attr_map &attrs)
+{
+ // Empty attributes are allowed and will just be encoded as {}.
Review Comment:
It's a bit of confused, why design it like this?
--
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]
