You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@pegasus.apache.org by GitBox <gi...@apache.org> on 2022/12/04 17:07:27 UTC

[GitHub] [incubator-pegasus] empiredan opened a new pull request, #1285: feat(new_metrics): traverse the whole registry to choose entities and metrics according to the filters

empiredan opened a new pull request, #1285:
URL: https://github.com/apache/incubator-pegasus/pull/1285

   This PR is to resolve #1279.


-- 
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


[GitHub] [incubator-pegasus] empiredan merged pull request #1285: feat(new_metrics): traverse the whole registry to choose entities and metrics according to the filters

Posted by GitBox <gi...@apache.org>.
empiredan merged PR #1285:
URL: https://github.com/apache/incubator-pegasus/pull/1285


-- 
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


[GitHub] [incubator-pegasus] acelyc111 commented on a diff in pull request #1285: feat(new_metrics): traverse the whole registry to choose entities and metrics according to the filters

Posted by GitBox <gi...@apache.org>.
acelyc111 commented on code in PR #1285:
URL: https://github.com/apache/incubator-pegasus/pull/1285#discussion_r1039824868


##########
src/utils/test/metrics_test.cpp:
##########
@@ -2178,4 +2178,121 @@ TEST(metrics_test, take_snapshot_entity)
     }
 }
 
+void check_entities_from_json_string(const std::string &json_string,
+                                     const std::unordered_set<std::string> &expected_entity_ids)
+{
+    // Even if there is not any entity selected, `json_string` should be "{}".
+    ASSERT_FALSE(json_string.empty());
+
+    rapidjson::Document doc;
+    rapidjson::ParseResult result = doc.Parse(json_string.c_str());
+    ASSERT_FALSE(result.IsError());
+
+    // Actual entity ids parsed from json string for entities.
+    std::unordered_set<std::string> actual_entity_ids;
+
+    // The json format for entities should be an array.
+    ASSERT_TRUE(doc.IsArray());
+    for (const auto &entity : doc.GetArray()) {
+        // The json format for each entity should be an object.
+        ASSERT_TRUE(entity.IsObject());
+
+        for (const auto &elem : entity.GetObject()) {
+            // Each name must be a string.
+            ASSERT_TRUE(elem.name.IsString());
+
+            if (kMetricEntityTypeField == elem.name.GetString()) {
+                ASSERT_STREQ(elem.value.GetString(), "my_server");
+            } else if (kMetricEntityIdField == elem.name.GetString()) {
+                actual_entity_ids.emplace(elem.value.GetString());
+            } else if (kMetricEntityAttrsField == elem.name.GetString()) {
+                ASSERT_TRUE(elem.value.IsObject());
+
+                // The attributes for each entity should be empty.
+                metric_entity::attr_map actual_entity_attrs;
+                for (const auto &attr : elem.value.GetObject()) {
+                    actual_entity_attrs.emplace(attr.name.GetString(), attr.value.GetString());
+                }
+                ASSERT_TRUE(actual_entity_attrs.empty());

Review Comment:
   Is it possible to use `ASSERT_TRUE(elem.value.ObjectEmpty());` ?



##########
src/utils/test/metrics_test.cpp:
##########
@@ -2178,4 +2178,121 @@ TEST(metrics_test, take_snapshot_entity)
     }
 }
 
+void check_entities_from_json_string(const std::string &json_string,
+                                     const std::unordered_set<std::string> &expected_entity_ids)
+{
+    // Even if there is not any entity selected, `json_string` should be "{}".
+    ASSERT_FALSE(json_string.empty());
+
+    rapidjson::Document doc;
+    rapidjson::ParseResult result = doc.Parse(json_string.c_str());
+    ASSERT_FALSE(result.IsError());
+
+    // Actual entity ids parsed from json string for entities.
+    std::unordered_set<std::string> actual_entity_ids;
+
+    // The json format for entities should be an array.
+    ASSERT_TRUE(doc.IsArray());
+    for (const auto &entity : doc.GetArray()) {
+        // The json format for each entity should be an object.
+        ASSERT_TRUE(entity.IsObject());
+
+        for (const auto &elem : entity.GetObject()) {
+            // Each name must be a string.
+            ASSERT_TRUE(elem.name.IsString());
+
+            if (kMetricEntityTypeField == elem.name.GetString()) {
+                ASSERT_STREQ(elem.value.GetString(), "my_server");
+            } else if (kMetricEntityIdField == elem.name.GetString()) {
+                actual_entity_ids.emplace(elem.value.GetString());
+            } else if (kMetricEntityAttrsField == elem.name.GetString()) {
+                ASSERT_TRUE(elem.value.IsObject());
+
+                // The attributes for each entity should be empty.
+                metric_entity::attr_map actual_entity_attrs;
+                for (const auto &attr : elem.value.GetObject()) {
+                    actual_entity_attrs.emplace(attr.name.GetString(), attr.value.GetString());
+                }
+                ASSERT_TRUE(actual_entity_attrs.empty());
+            } else if (kMetricEntityMetricsField == elem.name.GetString()) {
+                ASSERT_TRUE(elem.value.IsArray());
+
+                std::unordered_set<std::string> actual_entity_metrics;
+                for (const auto &m : elem.value.GetArray()) {
+                    ASSERT_TRUE(m.IsObject());
+
+                    for (const auto &field : m.GetObject()) {
+                        // Each name must be a string.
+                        ASSERT_TRUE(field.name.IsString());
+                        if (kMetricNameField == field.name.GetString()) {
+                            ASSERT_TRUE(field.value.IsString());
+                            actual_entity_metrics.emplace(field.value.GetString());
+                        }
+                    }
+                }
+
+                static const std::unordered_set<std::string> kExpectedEntityMetrics = {
+                    "test_gauge_int64", "test_counter"};
+                ASSERT_EQ(actual_entity_metrics, kExpectedEntityMetrics);

Review Comment:
   It's better to use it like `ASSERT_EQ(expect_value, actual_value);`



-- 
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


[GitHub] [incubator-pegasus] acelyc111 commented on a diff in pull request #1285: feat(new_metrics): traverse the whole registry to choose entities and metrics according to the filters

Posted by GitBox <gi...@apache.org>.
acelyc111 commented on code in PR #1285:
URL: https://github.com/apache/incubator-pegasus/pull/1285#discussion_r1039827780


##########
src/utils/test/metrics_test.cpp:
##########
@@ -2178,4 +2178,121 @@ TEST(metrics_test, take_snapshot_entity)
     }
 }
 
+void check_entities_from_json_string(const std::string &json_string,
+                                     const std::unordered_set<std::string> &expected_entity_ids)
+{
+    // Even if there is not any entity selected, `json_string` should be "{}".
+    ASSERT_FALSE(json_string.empty());
+
+    rapidjson::Document doc;
+    rapidjson::ParseResult result = doc.Parse(json_string.c_str());
+    ASSERT_FALSE(result.IsError());
+
+    // Actual entity ids parsed from json string for entities.
+    std::unordered_set<std::string> actual_entity_ids;
+
+    // The json format for entities should be an array.
+    ASSERT_TRUE(doc.IsArray());
+    for (const auto &entity : doc.GetArray()) {
+        // The json format for each entity should be an object.
+        ASSERT_TRUE(entity.IsObject());
+
+        for (const auto &elem : entity.GetObject()) {
+            // Each name must be a string.
+            ASSERT_TRUE(elem.name.IsString());
+
+            if (kMetricEntityTypeField == elem.name.GetString()) {
+                ASSERT_STREQ(elem.value.GetString(), "my_server");
+            } else if (kMetricEntityIdField == elem.name.GetString()) {
+                actual_entity_ids.emplace(elem.value.GetString());
+            } else if (kMetricEntityAttrsField == elem.name.GetString()) {
+                ASSERT_TRUE(elem.value.IsObject());
+
+                // The attributes for each entity should be empty.
+                metric_entity::attr_map actual_entity_attrs;
+                for (const auto &attr : elem.value.GetObject()) {
+                    actual_entity_attrs.emplace(attr.name.GetString(), attr.value.GetString());
+                }
+                ASSERT_TRUE(actual_entity_attrs.empty());
+            } else if (kMetricEntityMetricsField == elem.name.GetString()) {
+                ASSERT_TRUE(elem.value.IsArray());
+
+                std::unordered_set<std::string> actual_entity_metrics;
+                for (const auto &m : elem.value.GetArray()) {
+                    ASSERT_TRUE(m.IsObject());
+
+                    for (const auto &field : m.GetObject()) {
+                        // Each name must be a string.
+                        ASSERT_TRUE(field.name.IsString());
+                        if (kMetricNameField == field.name.GetString()) {
+                            ASSERT_TRUE(field.value.IsString());
+                            actual_entity_metrics.emplace(field.value.GetString());
+                        }
+                    }
+                }
+
+                static const std::unordered_set<std::string> kExpectedEntityMetrics = {
+                    "test_gauge_int64", "test_counter"};
+                ASSERT_EQ(actual_entity_metrics, kExpectedEntityMetrics);

Review Comment:
   It's better to use it like `ASSERT_EQ(expect_value, actual_value);` because of the error message is act like 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