[GitHub] [nifi-minifi-cpp] msharee9 commented on a change in pull request #734: MINIFICPP-1157 Implement lightweight C2 heartbeat.
msharee9 commented on a change in pull request #734: MINIFICPP-1157 Implement
lightweight C2 heartbeat.
URL: https://github.com/apache/nifi-minifi-cpp/pull/734#discussion_r385810699
##
File path: extensions/http-curl/tests/HTTPIntegrationBase.h
##
@@ -91,4 +87,74 @@ void CoapIntegrationBase::setUrl(std::string url,
CivetHandler *handler) {
}
}
+class VerifyC2Base : public CoapIntegrationBase {
+ public:
+ explicit VerifyC2Base(bool isSecure)
+ : isSecure(isSecure) {
+char format[] = "/tmp/ssth.XX";
+dir = testController.createTempDirectory(format);
+ }
+
+ virtual void testSetup() {
+LogTestController::getInstance().setDebug();
+LogTestController::getInstance().setDebug();
+std::fstream file;
+ss << dir << "/" << "tstFile.ext";
Review comment:
yes. We don't need them. Was a remnant from previous code. Removed it.
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [nifi-minifi-cpp] msharee9 commented on a change in pull request #734: MINIFICPP-1157 Implement lightweight C2 heartbeat.
msharee9 commented on a change in pull request #734: MINIFICPP-1157 Implement
lightweight C2 heartbeat.
URL: https://github.com/apache/nifi-minifi-cpp/pull/734#discussion_r385301812
##
File path: libminifi/include/core/state/nodes/AgentInformation.h
##
@@ -80,7 +80,7 @@ class ComponentManifest : public DeviceInformation {
return CoreComponent::getName();
}
- virtual std::vector serialize() {
+ virtual std::vector serialize() const {
Review comment:
ok. reverted.
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [nifi-minifi-cpp] msharee9 commented on a change in pull request #734: MINIFICPP-1157 Implement lightweight C2 heartbeat.
msharee9 commented on a change in pull request #734: MINIFICPP-1157 Implement
lightweight C2 heartbeat.
URL: https://github.com/apache/nifi-minifi-cpp/pull/734#discussion_r385301325
##
File path: extensions/http-curl/tests/HTTPHandlers.h
##
@@ -343,4 +345,104 @@ class DeleteTransactionResponder : public CivetHandler {
std::string response_code;
};
+class HeartbeatHandler : public CivetHandler {
+ public:
+ explicit HeartbeatHandler(bool isSecure)
+ : isSecure(isSecure) {
+ }
+
+ virtual ~HeartbeatHandler() = default;
+
+ std::string readPost(struct mg_connection *conn) {
+std::string response;
+int blockSize = 1024 * sizeof(char), readBytes;
+
+char buffer[1024];
+while ((readBytes = mg_read(conn, buffer, blockSize)) > 0) {
+ response.append(buffer, 0, (readBytes / sizeof(char)));
+}
+return response;
+ }
+
+ void sendStopOperation(struct mg_connection *conn) {
+std::string resp = "{\"operation\" : \"heartbeat\",
\"requested_operations\" : [{ \"operationid\" : 41, \"operation\" : \"stop\",
\"name\" : \"invoke\" }, "
+"{ \"operationid\" : 42, \"operation\" : \"stop\", \"name\" :
\"FlowController\" } ]}";
+mg_printf(conn, "HTTP/1.1 200 OK\r\nContent-Type: "
+ "text/plain\r\nContent-Length: %lu\r\nConnection: close\r\n\r\n",
+ resp.length());
+mg_printf(conn, "%s", resp.c_str());
+ }
+
+ void sendHeartbeatResponse(const std::string& operation, const std::string&
operand, const std::string& operationId, struct mg_connection * conn) {
+std::string heartbeat_response = "{\"operation\" :
\"heartbeat\",\"requested_operations\": [ {"
+ "\"operation\" : \"" + operation + "\","
+ "\"operationid\" : \"" + operationId + "\","
+ "\"operand\": \"" + operand + "\"}]}";
+
+ mg_printf(conn, "HTTP/1.1 200 OK\r\nContent-Type: "
+"text/plain\r\nContent-Length: %lu\r\nConnection:
close\r\n\r\n",
+heartbeat_response.length());
+ mg_printf(conn, "%s", heartbeat_response.c_str());
+ }
+
+ void verifyJsonHasAgentManifest(const rapidjson::Document& root) {
+bool found = false;
+assert(root.HasMember("agentInfo") == true);
+assert(root["agentInfo"].HasMember("agentManifest") == true);
+assert(root["agentInfo"]["agentManifest"].HasMember("bundles") == true);
+
+for (auto :
root["agentInfo"]["agentManifest"]["bundles"].GetArray()) {
+ assert(bundle.HasMember("artifact"));
+ std::string str = bundle["artifact"].GetString();
+ if (str == "minifi-system") {
+
+std::vector classes;
+for (auto :
bundle["componentManifest"]["processors"].GetArray()) {
+ classes.push_back(proc["type"].GetString());
+}
+
+auto group = minifi::BuildDescription::getClassDescriptions(str);
+for (auto proc : group.processors_) {
+ assert(std::find(classes.begin(), classes.end(), proc.class_name_)
!= std::end(classes));
+ found = true;
+}
+
+ }
+}
+assert(found == true);
+ }
+
+ virtual void handleHeartbeat(const rapidjson::Document& root, struct
mg_connection * conn) {
+(void)conn;
+verifyJsonHasAgentManifest(root);
+ }
+
+ virtual void handleAcknowledge(const rapidjson::Document& root) {
+ }
+
+ void verify(struct mg_connection *conn) {
+auto post_data = readPost(conn);
+std::cerr << post_data << std::endl;
+if (!IsNullOrEmpty(post_data)) {
+ rapidjson::Document root;
+ rapidjson::ParseResult ok = root.Parse(post_data.data(),
post_data.size());
+ std::string operation = root["operation"].GetString();
+ if (operation == "heartbeat") {
+handleHeartbeat(root, conn);
+ } else if (operation == "acknowledge") {
+handleAcknowledge(root);
+ }
Review comment:
Ok. Makes sense.
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [nifi-minifi-cpp] msharee9 commented on a change in pull request #734: MINIFICPP-1157 Implement lightweight C2 heartbeat.
msharee9 commented on a change in pull request #734: MINIFICPP-1157 Implement
lightweight C2 heartbeat.
URL: https://github.com/apache/nifi-minifi-cpp/pull/734#discussion_r385301665
##
File path: extensions/http-curl/tests/C2JstackTest.cpp
##
@@ -45,123 +45,114 @@
#include "CivetServer.h"
#include
#include "protocols/RESTSender.h"
+#include "HTTPIntegrationBase.h"
+#include "HTTPHandlers.h"
-void waitToVerifyProcessor() {
- std::this_thread::sleep_for(std::chrono::seconds(10));
-}
+class VerifyC2Describe : public VerifyC2Base {
+ public:
+ explicit VerifyC2Describe(bool isSecure)
+ : VerifyC2Base(isSecure) {
+ }
+ virtual ~VerifyC2Describe() = default;
-class ConfigHandler : public CivetHandler {
- public:
- ConfigHandler() {
-calls_ = 0;
+ void testSetup() {
+LogTestController::getInstance().setTrace();
+LogTestController::getInstance().setDebug();
+LogTestController::getInstance().setInfo();
+VerifyC2Base::testSetup();
}
- bool handlePost(CivetServer *server, struct mg_connection *conn) {
-calls_++;
-std::string heartbeat_response = "{\"operation\" :
\"heartbeat\",\"requested_operations\": [ {"
- "\"operation\" : \"describe\", "
- "\"operationid\" : \"8675309\", "
- "\"name\": \"jstack\""
- "}]}";
- mg_printf(conn, "HTTP/1.1 200 OK\r\nContent-Type: "
-"text/plain\r\nContent-Length: %lu\r\nConnection:
close\r\n\r\n",
-heartbeat_response.length());
- mg_printf(conn, "%s", heartbeat_response.c_str());
-
-
-return true;
+
+ void configureC2RootClasses() {
+configuration->set("nifi.c2.root.classes",
"DeviceInfoNode,AgentInformationWithoutManifest,FlowInformation");
}
+};
- bool handleGet(CivetServer *server, struct mg_connection *conn) {
-std::ifstream myfile(test_file_location_.c_str());
-
-if (myfile.is_open()) {
- std::stringstream buffer;
- buffer << myfile.rdbuf();
- std::string str = buffer.str();
- myfile.close();
- mg_printf(conn, "HTTP/1.1 200 OK\r\nContent-Type: "
-"text/plain\r\nContent-Length: %lu\r\nConnection:
close\r\n\r\n",
-str.length());
- mg_printf(conn, "%s", str.c_str());
-} else {
- mg_printf(conn, "HTTP/1.1 500 Internal Server Error\r\n");
-}
+class VerifyC2DescribeJstack : public VerifyC2Describe {
+ public:
+ explicit VerifyC2DescribeJstack(bool isSecure)
+ : VerifyC2Describe(isSecure) {
+ }
+
+ virtual ~VerifyC2DescribeJstack() = default;
-return true;
+ virtual void runAssertions() {
+assert(LogTestController::getInstance().contains("SchedulingAgent") ==
true);
}
- std::string test_file_location_;
- std::atomic calls_;
+
};
-int main(int argc, char **argv) {
- mg_init_library(0);
- LogTestController::getInstance().setInfo();
- LogTestController::getInstance().setDebug();
- LogTestController::getInstance().setDebug();
- LogTestController::getInstance().setTrace();
-
- const char *options[] = { "document_root", ".", "listening_ports", "0", 0 };
- std::vector cpp_options;
- for (int i = 0; i < (sizeof(options) / sizeof(options[0]) - 1); i++) {
-cpp_options.push_back(options[i]);
+class DescribeManifestHandler: public HeartbeatHandler {
Review comment:
Created another test file for DESCRIBE manifest
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [nifi-minifi-cpp] msharee9 commented on a change in pull request #734: MINIFICPP-1157 Implement lightweight C2 heartbeat.
msharee9 commented on a change in pull request #734: MINIFICPP-1157 Implement
lightweight C2 heartbeat.
URL: https://github.com/apache/nifi-minifi-cpp/pull/734#discussion_r385301458
##
File path: libminifi/include/core/state/nodes/MetricsBase.h
##
@@ -228,6 +228,20 @@ class NodeReporter {
*/
virtual int16_t getMetricsNodes(std::vector>
_vector, uint16_t metricsClass) = 0;
+ /**
+ * Retrieves agent information with manifest only from this source.
+ * @param manifest_vector -- manifest nodes vector.
+ * @return 0 on Success, -1 on failure
+ */
+ virtual int16_t
getManifestNodes(std::vector>&
manifest_vector) const = 0;
+
+ /**
+ * Returns a response node containing all agent information with manifest
and agent status
+ * @return a shared pointer to agent information
+ */
+ virtual std::shared_ptr getAgentInformation()
const {
+ return nullptr;
Review comment:
Made it pure virtual.
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [nifi-minifi-cpp] msharee9 commented on a change in pull request #734: MINIFICPP-1157 Implement lightweight C2 heartbeat.
msharee9 commented on a change in pull request #734: MINIFICPP-1157 Implement
lightweight C2 heartbeat.
URL: https://github.com/apache/nifi-minifi-cpp/pull/734#discussion_r385301018
##
File path: extensions/http-curl/tests/C2JstackTest.cpp
##
@@ -45,123 +45,114 @@
#include "CivetServer.h"
#include
#include "protocols/RESTSender.h"
+#include "HTTPIntegrationBase.h"
+#include "HTTPHandlers.h"
-void waitToVerifyProcessor() {
- std::this_thread::sleep_for(std::chrono::seconds(10));
-}
+class VerifyC2Describe : public VerifyC2Base {
+ public:
+ explicit VerifyC2Describe(bool isSecure)
+ : VerifyC2Base(isSecure) {
+ }
+ virtual ~VerifyC2Describe() = default;
-class ConfigHandler : public CivetHandler {
- public:
- ConfigHandler() {
-calls_ = 0;
+ void testSetup() {
+LogTestController::getInstance().setTrace();
+LogTestController::getInstance().setDebug();
+LogTestController::getInstance().setInfo();
+VerifyC2Base::testSetup();
}
- bool handlePost(CivetServer *server, struct mg_connection *conn) {
-calls_++;
-std::string heartbeat_response = "{\"operation\" :
\"heartbeat\",\"requested_operations\": [ {"
- "\"operation\" : \"describe\", "
- "\"operationid\" : \"8675309\", "
- "\"name\": \"jstack\""
- "}]}";
- mg_printf(conn, "HTTP/1.1 200 OK\r\nContent-Type: "
-"text/plain\r\nContent-Length: %lu\r\nConnection:
close\r\n\r\n",
-heartbeat_response.length());
- mg_printf(conn, "%s", heartbeat_response.c_str());
-
-
-return true;
+
+ void configureC2RootClasses() {
+configuration->set("nifi.c2.root.classes",
"DeviceInfoNode,AgentInformationWithoutManifest,FlowInformation");
}
+};
- bool handleGet(CivetServer *server, struct mg_connection *conn) {
-std::ifstream myfile(test_file_location_.c_str());
-
-if (myfile.is_open()) {
- std::stringstream buffer;
- buffer << myfile.rdbuf();
- std::string str = buffer.str();
- myfile.close();
- mg_printf(conn, "HTTP/1.1 200 OK\r\nContent-Type: "
-"text/plain\r\nContent-Length: %lu\r\nConnection:
close\r\n\r\n",
-str.length());
- mg_printf(conn, "%s", str.c_str());
-} else {
- mg_printf(conn, "HTTP/1.1 500 Internal Server Error\r\n");
-}
+class VerifyC2DescribeJstack : public VerifyC2Describe {
+ public:
+ explicit VerifyC2DescribeJstack(bool isSecure)
+ : VerifyC2Describe(isSecure) {
+ }
+
+ virtual ~VerifyC2DescribeJstack() = default;
-return true;
+ virtual void runAssertions() {
+assert(LogTestController::getInstance().contains("SchedulingAgent") ==
true);
}
- std::string test_file_location_;
- std::atomic calls_;
+
};
-int main(int argc, char **argv) {
- mg_init_library(0);
- LogTestController::getInstance().setInfo();
- LogTestController::getInstance().setDebug();
- LogTestController::getInstance().setDebug();
- LogTestController::getInstance().setTrace();
-
- const char *options[] = { "document_root", ".", "listening_ports", "0", 0 };
- std::vector cpp_options;
- for (int i = 0; i < (sizeof(options) / sizeof(options[0]) - 1); i++) {
-cpp_options.push_back(options[i]);
+class DescribeManifestHandler: public HeartbeatHandler {
+public:
+
+ explicit DescribeManifestHandler(bool isSecure)
+ : HeartbeatHandler(isSecure) {
}
- CivetServer server(cpp_options);
+ virtual ~DescribeManifestHandler() = default;
Review comment:
Valid point.
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [nifi-minifi-cpp] msharee9 commented on a change in pull request #734: MINIFICPP-1157 Implement lightweight C2 heartbeat.
msharee9 commented on a change in pull request #734: MINIFICPP-1157 Implement
lightweight C2 heartbeat.
URL: https://github.com/apache/nifi-minifi-cpp/pull/734#discussion_r385300910
##
File path: extensions/coap/tests/CoapIntegrationBase.h
##
@@ -41,13 +41,13 @@ class CoapIntegrationBase : public IntegrationBase {
void setUrl(std::string url, CivetHandler *handler);
- virtual ~CoapIntegrationBase();
+ virtual ~CoapIntegrationBase() = default;
void shutdownBeforeFlowController() override {
stop_webserver(server);
}
- virtual void run(std::string test_file_location) override {
Review comment:
May have removed it accidentally. Will put it back.
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [nifi-minifi-cpp] msharee9 commented on a change in pull request #734: MINIFICPP-1157 Implement lightweight C2 heartbeat.
msharee9 commented on a change in pull request #734: MINIFICPP-1157 Implement
lightweight C2 heartbeat.
URL: https://github.com/apache/nifi-minifi-cpp/pull/734#discussion_r385300629
##
File path: conf/minifi.properties
##
@@ -51,7 +51,7 @@
nifi.database.content.repository.directory.default=${MINIFI_HOME}/content_reposi
#nifi.c2.flow.base.url=
#nifi.c2.rest.url=
#nifi.c2.rest.url.ack=
-nifi.c2.root.classes=DeviceInfoNode,AgentInformation,FlowInformation
+nifi.c2.root.classes=DeviceInfoNode,AgentInformationWithoutManifest,FlowInformation
Review comment:
Ok
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [nifi-minifi-cpp] msharee9 commented on a change in pull request #734: MINIFICPP-1157 Implement lightweight C2 heartbeat.
msharee9 commented on a change in pull request #734: MINIFICPP-1157 Implement
lightweight C2 heartbeat.
URL: https://github.com/apache/nifi-minifi-cpp/pull/734#discussion_r382158762
##
File path: libminifi/src/c2/C2Agent.cpp
##
@@ -315,16 +317,35 @@ void C2Agent::performHeartBeat() {
payload.addPayload(std::move(deviceInfo));
}
- if (!root_response_nodes_.empty()) {
-for (auto metric : root_response_nodes_) {
- C2Payload child_metric_payload(Operation::HEARTBEAT);
- child_metric_payload.setLabel(metric.first);
- if (metric.second->isArray()) {
-child_metric_payload.setContainer(true);
- }
- serializeMetrics(child_metric_payload, metric.first,
metric.second->serialize(), metric.second->isArray());
- payload.addPayload(std::move(child_metric_payload));
+ for (auto metric : root_response_nodes_) {
+C2Payload child_metric_payload(Operation::HEARTBEAT);
+bool isArray{false};
+std::string metricName;
+std::vector metrics;
+std::shared_ptr reporter;
+std::shared_ptr agentInfoManifest;
+
+//Send agent manifest in first heartbeat
Review comment:
C2 refactor design doc work on its way. Will create tickets after we review
the doc.
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [nifi-minifi-cpp] msharee9 commented on a change in pull request #734: MINIFICPP-1157 Implement lightweight C2 heartbeat.
msharee9 commented on a change in pull request #734: MINIFICPP-1157 Implement
lightweight C2 heartbeat.
URL: https://github.com/apache/nifi-minifi-cpp/pull/734#discussion_r382143961
##
File path: libminifi/src/FlowController.cpp
##
@@ -931,6 +942,35 @@ int16_t
FlowController::getMetricsNodes(std::vector>&
manifest_vector, uint16_t metricsClass) {
Review comment:
Makes sense. Made it const
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [nifi-minifi-cpp] msharee9 commented on a change in pull request #734: MINIFICPP-1157 Implement lightweight C2 heartbeat.
msharee9 commented on a change in pull request #734: MINIFICPP-1157 Implement
lightweight C2 heartbeat.
URL: https://github.com/apache/nifi-minifi-cpp/pull/734#discussion_r379564619
##
File path: libminifi/src/c2/C2Agent.cpp
##
@@ -315,16 +317,35 @@ void C2Agent::performHeartBeat() {
payload.addPayload(std::move(deviceInfo));
}
- if (!root_response_nodes_.empty()) {
-for (auto metric : root_response_nodes_) {
- C2Payload child_metric_payload(Operation::HEARTBEAT);
- child_metric_payload.setLabel(metric.first);
- if (metric.second->isArray()) {
-child_metric_payload.setContainer(true);
- }
- serializeMetrics(child_metric_payload, metric.first,
metric.second->serialize(), metric.second->isArray());
- payload.addPayload(std::move(child_metric_payload));
+ for (auto metric : root_response_nodes_) {
+C2Payload child_metric_payload(Operation::HEARTBEAT);
+bool isArray{false};
+std::string metricName;
+std::vector metrics;
+std::shared_ptr reporter;
+std::shared_ptr agentInfoManifest;
+
+//Send agent manifest in first heartbeat
Review comment:
Replying to your first part of the comment, I agree with you that the
registration of C2Agent and the code involving setting and getting response
nodes is not very clean and difficult to understand. From my understanding,
FlowController is NodeReporter (metrics source) and C2Agent is ResponseNodeSink
(metrics sink). The TreeUpdateListener runs a loop in a thread I believe which
constantly updates the sink cache with metrics and I think this is unnecessary
optimization.
Up until now, the FlowController was responsible for instantiating classes
that were defined in the nifi.c2.root.classes property, exposed via
getResponseNodes and retrieved by the C2Agent when needed. The change I made
also exposes the class via FlowController, this time without it being defined
in the properties but that is out of necessity for optimization. (I am talking
about first full heartbeat). I agree that the behavior wise this is a little
hacky that we are now making a behavioral change in C2Agent rather than making
that decision in the metrics classes themselves but this is because there is no
way of knowing about the C2Agent state in those classes as you mentioned.
We definitely need a cleaner design especially surrounding the structure of
metrics classes and the source and sink concepts. I believe may be a visitor
approach could make this design much simpler.
Having said that, we can still remove the first heartbeat optimization in
lieu of putting back the behavior in the metrics classes whichever is a better
tradeoff. What do you think?
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [nifi-minifi-cpp] msharee9 commented on a change in pull request #734: MINIFICPP-1157 Implement lightweight C2 heartbeat.
msharee9 commented on a change in pull request #734: MINIFICPP-1157 Implement
lightweight C2 heartbeat.
URL: https://github.com/apache/nifi-minifi-cpp/pull/734#discussion_r379166388
##
File path: libminifi/src/c2/C2Agent.cpp
##
@@ -539,9 +563,9 @@ void C2Agent::handle_describe(const C2ContentResponse
) {
} else if (resp.name == "manifest") {
auto keys = configuration_->getConfiguredKeys();
C2Payload response(Operation::ACKNOWLEDGE, resp.ident, false, true);
-response.setLabel("configuration_options");
+response.setLabel("configurationOptions");
Review comment:
For jstack "configuration_options" does not appear in the response payload.
The inner payload label matters while serializing json. That is what I found
out while testing DESCRIBE jstack.
On the side note, the DESCRIBE operand should be pstack I believe for a
better name for C++. Looks like jstack was taken from Minifi Java.
Nonetheless, removed that label for jstack response.
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [nifi-minifi-cpp] msharee9 commented on a change in pull request #734: MINIFICPP-1157 Implement lightweight C2 heartbeat.
msharee9 commented on a change in pull request #734: MINIFICPP-1157 Implement
lightweight C2 heartbeat.
URL: https://github.com/apache/nifi-minifi-cpp/pull/734#discussion_r379159050
##
File path: libminifi/src/c2/C2Agent.cpp
##
@@ -539,9 +563,9 @@ void C2Agent::handle_describe(const C2ContentResponse
) {
} else if (resp.name == "manifest") {
auto keys = configuration_->getConfiguredKeys();
Review comment:
good catch. Will make the necessary corrections here.
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [nifi-minifi-cpp] msharee9 commented on a change in pull request #734: MINIFICPP-1157 Implement lightweight C2 heartbeat.
msharee9 commented on a change in pull request #734: MINIFICPP-1157 Implement
lightweight C2 heartbeat.
URL: https://github.com/apache/nifi-minifi-cpp/pull/734#discussion_r379150417
##
File path: libminifi/src/c2/C2Agent.cpp
##
@@ -506,13 +527,16 @@ void C2Agent::handle_describe(const C2ContentResponse
) {
}
std::vector> metrics_vec;
-
- reporter->getResponseNodes(metrics_vec, metric_class_id);
C2Payload response(Operation::ACKNOWLEDGE, resp.ident, false, true);
response.setLabel("metrics");
+
+ C2Payload metrics(Operation::ACKNOWLEDGE, resp.ident, false, true);
Review comment:
That is because of the way C2Payload and C2ContentResponse classes are
structured and the way we serializeJson payload. This needs some cleanup and
might require some larger effort. Will take a note and open a Jira ticket to
simplify this design.
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [nifi-minifi-cpp] msharee9 commented on a change in pull request #734: MINIFICPP-1157 Implement lightweight C2 heartbeat.
msharee9 commented on a change in pull request #734: MINIFICPP-1157 Implement
lightweight C2 heartbeat.
URL: https://github.com/apache/nifi-minifi-cpp/pull/734#discussion_r379042436
##
File path: libminifi/src/c2/C2Agent.cpp
##
@@ -521,9 +545,9 @@ void C2Agent::handle_describe(const C2ContentResponse
) {
std::vector keys;
std::copy_if(unsanitized_keys.begin(), unsanitized_keys.end(),
std::back_inserter(keys), [](std::string key) {return key.find("pass") ==
std::string::npos;});
C2Payload response(Operation::ACKNOWLEDGE, resp.ident, false, true);
-response.setLabel("configuration_options");
+response.setLabel("configurationOptions");
Review comment:
Agree. The wiki doc is very much out of date and we cannot tell from that if
someone might be using it. Will revert the label name.
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [nifi-minifi-cpp] msharee9 commented on a change in pull request #734: MINIFICPP-1157 Implement lightweight C2 heartbeat.
msharee9 commented on a change in pull request #734: MINIFICPP-1157 Implement
lightweight C2 heartbeat.
URL: https://github.com/apache/nifi-minifi-cpp/pull/734#discussion_r378996918
##
File path: libminifi/src/c2/C2Agent.cpp
##
@@ -315,16 +317,35 @@ void C2Agent::performHeartBeat() {
payload.addPayload(std::move(deviceInfo));
}
- if (!root_response_nodes_.empty()) {
-for (auto metric : root_response_nodes_) {
- C2Payload child_metric_payload(Operation::HEARTBEAT);
- child_metric_payload.setLabel(metric.first);
- if (metric.second->isArray()) {
-child_metric_payload.setContainer(true);
- }
- serializeMetrics(child_metric_payload, metric.first,
metric.second->serialize(), metric.second->isArray());
- payload.addPayload(std::move(child_metric_payload));
+ for (auto metric : root_response_nodes_) {
+C2Payload child_metric_payload(Operation::HEARTBEAT);
+bool isArray{false};
+std::string metricName;
+std::vector metrics;
+std::shared_ptr reporter;
+std::shared_ptr agentInfoManifest;
+
+//Send agent manifest in first heartbeat
Review comment:
When AgentInformation class is configured, this code uses AgentInformation
class, not AgentInformationWithManifest.
AgentInformation class has been modified structurally but, when you
instantiate and serialize it, it gives you the full heartbeat including
manifest. This class is kept here to maintain backwards compatibility.
May be the function "getAgentInformationWithManifest" name is confusing to
you. If you look at this function, it instantiates "AgentInformation" class.
Will change the name to "getAgentInformation" instead.
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [nifi-minifi-cpp] msharee9 commented on a change in pull request #734: MINIFICPP-1157 Implement lightweight C2 heartbeat.
msharee9 commented on a change in pull request #734: MINIFICPP-1157 Implement
lightweight C2 heartbeat.
URL: https://github.com/apache/nifi-minifi-cpp/pull/734#discussion_r378537631
##
File path: libminifi/src/FlowController.cpp
##
@@ -931,6 +942,35 @@ int16_t
FlowController::getMetricsNodes(std::vector>&
manifest_vector, uint16_t metricsClass) {
+std::lock_guard lock(metrics_mutex_);
+(void)metricsClass;
Review comment:
Are you talking about the "metricsClass" variable there?
If you are asking about the function, it returns manifest information needed
while responding to DESCRIBE manifest.
metricsClass has been removed from this function in the updated code.
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [nifi-minifi-cpp] msharee9 commented on a change in pull request #734: MINIFICPP-1157 Implement lightweight C2 heartbeat.
msharee9 commented on a change in pull request #734: MINIFICPP-1157 Implement
lightweight C2 heartbeat.
URL: https://github.com/apache/nifi-minifi-cpp/pull/734#discussion_r378537631
##
File path: libminifi/src/FlowController.cpp
##
@@ -931,6 +942,35 @@ int16_t
FlowController::getMetricsNodes(std::vector>&
manifest_vector, uint16_t metricsClass) {
+std::lock_guard lock(metrics_mutex_);
+(void)metricsClass;
Review comment:
This is a cache of manifest object (not the actual manifest data). Needed in
response to DESCRIBE manifest.
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [nifi-minifi-cpp] msharee9 commented on a change in pull request #734: MINIFICPP-1157 Implement lightweight C2 heartbeat.
msharee9 commented on a change in pull request #734: MINIFICPP-1157 Implement
lightweight C2 heartbeat.
URL: https://github.com/apache/nifi-minifi-cpp/pull/734#discussion_r378529132
##
File path: libminifi/include/core/state/nodes/AgentInformation.h
##
@@ -643,16 +704,7 @@ class AgentInformation : public DeviceInformation, public
AgentMonitor, public A
}
std::vector serialize() {
-std::vector serialized;
-
-SerializedResponseNode ident;
-
-ident.name = "identifier";
-ident.value = identifier_;
-
-SerializedResponseNode agentClass;
-agentClass.name = "agentClass";
-agentClass.value = agent_class_;
+std::vector
serialized(std::move(AgentInformationWithoutManifest::serialize()));
Review comment:
good point. Will remove it
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [nifi-minifi-cpp] msharee9 commented on a change in pull request #734: MINIFICPP-1157 Implement lightweight C2 heartbeat.
msharee9 commented on a change in pull request #734: MINIFICPP-1157 Implement
lightweight C2 heartbeat.
URL: https://github.com/apache/nifi-minifi-cpp/pull/734#discussion_r378462225
##
File path: libminifi/src/FlowController.cpp
##
@@ -931,6 +942,35 @@ int16_t
FlowController::getMetricsNodes(std::vector>&
manifest_vector, uint16_t metricsClass) {
+std::lock_guard lock(metrics_mutex_);
+(void)metricsClass;
+for (auto metric : agent_information_) {
Review comment:
ok
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [nifi-minifi-cpp] msharee9 commented on a change in pull request #734: MINIFICPP-1157 Implement lightweight C2 heartbeat.
msharee9 commented on a change in pull request #734: MINIFICPP-1157 Implement
lightweight C2 heartbeat.
URL: https://github.com/apache/nifi-minifi-cpp/pull/734#discussion_r378462016
##
File path: libminifi/src/c2/C2Agent.cpp
##
@@ -315,16 +317,35 @@ void C2Agent::performHeartBeat() {
payload.addPayload(std::move(deviceInfo));
}
- if (!root_response_nodes_.empty()) {
-for (auto metric : root_response_nodes_) {
- C2Payload child_metric_payload(Operation::HEARTBEAT);
- child_metric_payload.setLabel(metric.first);
- if (metric.second->isArray()) {
-child_metric_payload.setContainer(true);
- }
- serializeMetrics(child_metric_payload, metric.first,
metric.second->serialize(), metric.second->isArray());
- payload.addPayload(std::move(child_metric_payload));
+ for (auto metric : root_response_nodes_) {
+C2Payload child_metric_payload(Operation::HEARTBEAT);
+bool isArray{false};
+std::string metricName;
+std::vector metrics;
+std::shared_ptr reporter;
+std::shared_ptr agentInfoManifest;
+
+//Send agent manifest in first heartbeat
+if (!manifest_sent_
+&& (reporter =
std::dynamic_pointer_cast(update_sink_))
+&& (agentInfoManifest = reporter->getAgentInformationWithManifest())
+&& metric.first == agentInfoManifest->getName()) {
+
+metricName = agentInfoManifest->getName();
+isArray = agentInfoManifest->isArray();
+metrics = std::move(agentInfoManifest->serialize());
+manifest_sent_ = true;
Review comment:
If C2 server does not receive the manifest in the first heartbeat, it
requests manifest in DESCRIBE command when it next receives lightweight
heartbeat. Sending the manifest during the first heartbeat is a slight
optimization.
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [nifi-minifi-cpp] msharee9 commented on a change in pull request #734: MINIFICPP-1157 Implement lightweight C2 heartbeat.
msharee9 commented on a change in pull request #734: MINIFICPP-1157 Implement
lightweight C2 heartbeat.
URL: https://github.com/apache/nifi-minifi-cpp/pull/734#discussion_r378458847
##
File path: libminifi/include/core/state/nodes/AgentInformation.h
##
@@ -623,18 +632,70 @@ class AgentManifest : public DeviceInformation {
};
/**
- * Purpose and Justification: Prints classes along with their properties for
the current agent.
+ * This class is used for regular heartbeat without manifest
+ * A light weight heartbeat
*/
-class AgentInformation : public DeviceInformation, public AgentMonitor, public
AgentIdentifier {
+class AgentInformationWithoutManifest : public DeviceInformation, public
AgentMonitor, public AgentIdentifier {
+public:
+
+AgentInformationWithoutManifest(std::string name, utils::Identifier & uuid)
+: DeviceInformation(name, uuid) {
+ setArray(false);
+ }
+
+AgentInformationWithoutManifest(const std::string )
Review comment:
fair point. Thanks for catching it.
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [nifi-minifi-cpp] msharee9 commented on a change in pull request #734: MINIFICPP-1157 Implement lightweight C2 heartbeat.
msharee9 commented on a change in pull request #734: MINIFICPP-1157 Implement
lightweight C2 heartbeat.
URL: https://github.com/apache/nifi-minifi-cpp/pull/734#discussion_r378443443
##
File path: libminifi/src/c2/C2Agent.cpp
##
@@ -553,6 +577,20 @@ void C2Agent::handle_describe(const C2ContentResponse
) {
}
response.addPayload(std::move(options));
+auto reporter =
std::dynamic_pointer_cast(update_sink_);
+if (reporter != nullptr) {
+std::vector>
metrics_vec;
+
+C2Payload agentInfo(Operation::ACKNOWLEDGE, resp.ident, false, true);
+agentInfo.setLabel("agentInfo");
+
+reporter->getManifestNodes(metrics_vec, 0);
+for (auto metric : metrics_vec) {
Review comment:
fair 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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [nifi-minifi-cpp] msharee9 commented on a change in pull request #734: MINIFICPP-1157 Implement lightweight C2 heartbeat.
msharee9 commented on a change in pull request #734: MINIFICPP-1157 Implement
lightweight C2 heartbeat.
URL: https://github.com/apache/nifi-minifi-cpp/pull/734#discussion_r378441353
##
File path: libminifi/include/core/state/nodes/AgentInformation.h
##
@@ -623,18 +632,70 @@ class AgentManifest : public DeviceInformation {
};
/**
- * Purpose and Justification: Prints classes along with their properties for
the current agent.
+ * This class is used for regular heartbeat without manifest
+ * A light weight heartbeat
*/
-class AgentInformation : public DeviceInformation, public AgentMonitor, public
AgentIdentifier {
+class AgentInformationWithoutManifest : public DeviceInformation, public
AgentMonitor, public AgentIdentifier {
Review comment:
Did not understand why it does not fit is-a relationship to any of its base
classes. Can you give example?
No this cannot become a free function unless we change the way we register
and load classes.
This instantiation of this class and other class (AgentInformation) depends
on configuration of root node classes in "nifi.c2.root.classes" property that
are loaded during C2 initialization in FlowController. I want to keep it that
way and be consistent with the existing code base.
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
