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<state::response::SerializedResponseNode> metrics;
+ std::shared_ptr<state::response::NodeReporter> reporter;
+ std::shared_ptr<state::response::ResponseNode> 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
