[GitHub] [nifi-minifi-cpp] adamdebreceni commented on a change in pull request #1057: MINIFICPP-1537 - Log heartbeats on demand
adamdebreceni commented on a change in pull request #1057:
URL: https://github.com/apache/nifi-minifi-cpp/pull/1057#discussion_r635310034
##
File path: libminifi/include/core/logging/Logger.h
##
@@ -68,13 +69,32 @@ inline T conditional_conversion(T t) {
}
template
-inline std::string format_string(char const* format_str, Args&&... args) {
- char buf[LOG_BUFFER_SIZE];
- std::snprintf(buf, LOG_BUFFER_SIZE, format_str,
conditional_conversion(std::forward(args))...);
- return std::string(buf);
+inline std::string format_string(int max_size, char const* format_str,
Args&&... args) {
+ // try to use static buffer
+ char buf[LOG_BUFFER_SIZE + 1];
+ int result = std::snprintf(buf, LOG_BUFFER_SIZE + 1, format_str,
conditional_conversion(std::forward(args))...);
+ if (result < 0) {
+return std::string("Error while formatting log message");
+ }
+ if (result <= LOG_BUFFER_SIZE) {
+// static buffer was large enough
+return std::string(buf, result);
+ }
+ if (max_size >= 0 && max_size <= LOG_BUFFER_SIZE) {
+// static buffer was already larger than allowed, use the filled buffer
+return std::string(buf, LOG_BUFFER_SIZE);
+ }
+ // try to use dynamic buffer
+ size_t dynamic_buffer_size = max_size < 0 ? result : std::min(result,
max_size);
+ std::vector buffer(dynamic_buffer_size + 1); // extra '\0' character
+ result = std::snprintf(buffer.data(), buffer.size(), format_str,
conditional_conversion(std::forward(args))...);
Review comment:
this thought crossed my mind, but this explicitly says that modifying
through the const overload is undefined behavior, I would wait until C++17
--
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
[GitHub] [nifi-minifi-cpp] adamdebreceni commented on a change in pull request #1057: MINIFICPP-1537 - Log heartbeats on demand
adamdebreceni commented on a change in pull request #1057:
URL: https://github.com/apache/nifi-minifi-cpp/pull/1057#discussion_r635306229
##
File path: libminifi/src/Configure.cpp
##
@@ -37,6 +37,10 @@ bool Configure::get(const std::string& key, std::string&
value) const {
bool Configure::get(const std::string& key, const std::string& alternate_key,
std::string& value) const {
if (get(key, value)) {
+if (get(alternate_key)) {
+ const auto logger = logging::LoggerFactory::getLogger();
+ logger->log_warn("Both the property '%s' and an alternate property '%s'
are set. Using '%s'.", key, alternate_key, key);
Review comment:
see `C2.md`:
> Release 0.6.0: Please note that all c2 properties now exist as
`nifi.c2.*`. If your configuration properties files contain the former naming
convention of `c2.*`, we will continue to support that as an alternate key, but
you are encouraged to switch your configuration options as soon as possible.
although it does not say "deprecated" per se
--
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
[GitHub] [nifi-minifi-cpp] adamdebreceni commented on a change in pull request #1057: MINIFICPP-1537 - Log heartbeats on demand
adamdebreceni commented on a change in pull request #1057:
URL: https://github.com/apache/nifi-minifi-cpp/pull/1057#discussion_r635265649
##
File path: libminifi/include/utils/Enum.h
##
@@ -60,6 +63,22 @@ namespace utils {
#define FOR_EACH_8(fn, delim, _1, _2, _3, _4, _5, _6, _7, _8) \
fn(_1) delim() fn(_2) delim() fn(_3) delim() fn(_4) delim() \
fn(_5) delim() fn(_6) delim() fn(_7) delim() fn(_8)
+#define FOR_EACH_9(fn, delim, _1, _2, _3, _4, _5, _6, _7, _8, _9) \
+ fn(_1) delim() fn(_2) delim() fn(_3) delim() fn(_4) delim() \
+ fn(_5) delim() fn(_6) delim() fn(_7) delim() fn(_8) delim() \
+ fn(_9)
+#define FOR_EACH_10(fn, delim, _1, _2, _3, _4, _5, _6, _7, _8, _9, _10) \
+ fn(_1) delim() fn(_2) delim() fn(_3) delim() fn(_4) delim() \
+ fn(_5) delim() fn(_6) delim() fn(_7) delim() fn(_8) delim() \
+ fn(_9) delim() fn(_10)
+#define FOR_EACH_11(fn, delim, _1, _2, _3, _4, _5, _6, _7, _8, _9, _10, _11) \
+ fn(_1) delim() fn(_2) delim() fn(_3) delim() fn(_4) delim() \
+ fn(_5) delim() fn(_6) delim() fn(_7) delim() fn(_8) delim() \
+ fn(_9) delim() fn(_10) delim() fn(_11)
+#define FOR_EACH_12(fn, delim, _1, _2, _3, _4, _5, _6, _7, _8, _9, _10, _11,
_12) \
+ fn(_1) delim() fn(_2) delim() fn(_3) delim() fn(_4) delim() \
+ fn(_5) delim() fn(_6) delim() fn(_7) delim() fn(_8) delim() \
+ fn(_9) delim() fn(_10) delim() fn(_11) delim() fn(_12)
Review comment:
I cannot recall what was the reasoning behind the current
non-"recursive" macros, changed them
--
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
[GitHub] [nifi-minifi-cpp] adamdebreceni commented on a change in pull request #1057: MINIFICPP-1537 - Log heartbeats on demand
adamdebreceni commented on a change in pull request #1057:
URL: https://github.com/apache/nifi-minifi-cpp/pull/1057#discussion_r635264523
##
File path: libminifi/src/c2/HeartbeatJsonSerializer.cpp
##
@@ -0,0 +1,333 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+#include "c2/HeartbeatJsonSerializer.h"
+#include "rapidjson/document.h"
+#include "rapidjson/writer.h"
+#include "rapidjson/stringbuffer.h"
+#include "rapidjson/prettywriter.h"
+
+namespace org {
+namespace apache {
+namespace nifi {
+namespace minifi {
+namespace c2 {
+
+std::string HeartbeatJsonSerializer::getOperation(const C2Payload& payload) {
+ switch (payload.getOperation()) {
+case Operation::ACKNOWLEDGE:
+ return "acknowledge";
+case Operation::HEARTBEAT:
+ return "heartbeat";
+case Operation::RESTART:
+ return "restart";
+case Operation::DESCRIBE:
+ return "describe";
+case Operation::STOP:
+ return "stop";
+case Operation::START:
+ return "start";
+case Operation::UPDATE:
+ return "update";
+case Operation::PAUSE:
+ return "pause";
+case Operation::RESUME:
+ return "resume";
+default:
+ return "heartbeat";
+ }
+}
+
+static void serializeOperationInfo(rapidjson::Value& target, const C2Payload&
payload, rapidjson::Document::AllocatorType& alloc) {
+ gsl_Expects(target.IsObject());
+
+ target.AddMember("operation",
HeartbeatJsonSerializer::getOperation(payload), alloc);
+
+ std::string id = payload.getIdentifier();
+ if (id.empty()) {
+return;
+ }
+
+ target.AddMember("operationId", id, alloc);
+ std::string state_str;
+ switch (payload.getStatus().getState()) {
+case state::UpdateState::FULLY_APPLIED:
+ state_str = "FULLY_APPLIED";
+ break;
+case state::UpdateState::PARTIALLY_APPLIED:
+ state_str = "PARTIALLY_APPLIED";
+ break;
+case state::UpdateState::READ_ERROR:
+ state_str = "OPERATION_NOT_UNDERSTOOD";
+ break;
+case state::UpdateState::SET_ERROR:
+default:
+ state_str = "NOT_APPLIED";
+ }
Review comment:
agree, done
--
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
[GitHub] [nifi-minifi-cpp] adamdebreceni commented on a change in pull request #1057: MINIFICPP-1537 - Log heartbeats on demand
adamdebreceni commented on a change in pull request #1057:
URL: https://github.com/apache/nifi-minifi-cpp/pull/1057#discussion_r635264297
##
File path: libminifi/src/Configure.cpp
##
@@ -37,6 +37,10 @@ bool Configure::get(const std::string& key, std::string&
value) const {
bool Configure::get(const std::string& key, const std::string& alternate_key,
std::string& value) const {
if (get(key, value)) {
+if (get(alternate_key)) {
+ const auto logger = logging::LoggerFactory::getLogger();
+ logger->log_warn("Both the property '%s' and an alternate property '%s'
are set. Using '%s'.", key, alternate_key, key);
Review comment:
I was under the impression that these "non-nifi-prefixed" property names
are deprecated and will be removed in the future, as the user can specify these
"ignored" properties I feel like the fact that we are discarding them is worthy
of their attention
##
File path: libminifi/src/c2/HeartbeatJsonSerializer.cpp
##
@@ -0,0 +1,333 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+#include "c2/HeartbeatJsonSerializer.h"
+#include "rapidjson/document.h"
+#include "rapidjson/writer.h"
+#include "rapidjson/stringbuffer.h"
+#include "rapidjson/prettywriter.h"
+
+namespace org {
+namespace apache {
+namespace nifi {
+namespace minifi {
+namespace c2 {
+
+std::string HeartbeatJsonSerializer::getOperation(const C2Payload& payload) {
+ switch (payload.getOperation()) {
+case Operation::ACKNOWLEDGE:
+ return "acknowledge";
+case Operation::HEARTBEAT:
+ return "heartbeat";
+case Operation::RESTART:
+ return "restart";
+case Operation::DESCRIBE:
+ return "describe";
+case Operation::STOP:
+ return "stop";
+case Operation::START:
+ return "start";
+case Operation::UPDATE:
+ return "update";
+case Operation::PAUSE:
+ return "pause";
+case Operation::RESUME:
+ return "resume";
+default:
+ return "heartbeat";
+ }
+}
+
+static void serializeOperationInfo(rapidjson::Value& target, const C2Payload&
payload, rapidjson::Document::AllocatorType& alloc) {
+ gsl_Expects(target.IsObject());
+
+ target.AddMember("operation",
HeartbeatJsonSerializer::getOperation(payload), alloc);
+
+ std::string id = payload.getIdentifier();
+ if (id.empty()) {
+return;
+ }
+
+ target.AddMember("operationId", id, alloc);
+ std::string state_str;
+ switch (payload.getStatus().getState()) {
+case state::UpdateState::FULLY_APPLIED:
+ state_str = "FULLY_APPLIED";
+ break;
+case state::UpdateState::PARTIALLY_APPLIED:
+ state_str = "PARTIALLY_APPLIED";
+ break;
+case state::UpdateState::READ_ERROR:
+ state_str = "OPERATION_NOT_UNDERSTOOD";
+ break;
+case state::UpdateState::SET_ERROR:
+default:
+ state_str = "NOT_APPLIED";
+ }
Review comment:
done
--
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
[GitHub] [nifi-minifi-cpp] adamdebreceni commented on a change in pull request #1057: MINIFICPP-1537 - Log heartbeats on demand
adamdebreceni commented on a change in pull request #1057:
URL: https://github.com/apache/nifi-minifi-cpp/pull/1057#discussion_r635261808
##
File path: libminifi/include/core/logging/Logger.h
##
@@ -68,13 +69,32 @@ inline T conditional_conversion(T t) {
}
template
-inline std::string format_string(char const* format_str, Args&&... args) {
- char buf[LOG_BUFFER_SIZE];
- std::snprintf(buf, LOG_BUFFER_SIZE, format_str,
conditional_conversion(std::forward(args))...);
- return std::string(buf);
+inline std::string format_string(int max_size, char const* format_str,
Args&&... args) {
+ if (0 <= max_size && max_size <= LOG_BUFFER_SIZE) {
+// use static buffer
+char buf[LOG_BUFFER_SIZE + 1];
+std::snprintf(buf, max_size + 1, format_str,
conditional_conversion(std::forward(args))...);
+return std::string(buf);
+ }
+ // use dynamically allocated buffer
+ if (max_size < 0) {
+// query what buffer size we need
+int size_needed = std::snprintf(nullptr, 0, format_str,
conditional_conversion(std::forward(args))...);
+if (size_needed < 0) {
+ // error
+ return std::string("Error while formatting log message");
+}
+std::vector buffer(size_needed + 1); // extra '\0' character
+std::snprintf(buffer.data(), buffer.size(), format_str,
conditional_conversion(std::forward(args))...);
+return std::string(buffer.data());
+ }
+ // use dynamic but fix-sized buffer
+ std::vector buffer(max_size);
+ std::snprintf(buffer.data(), buffer.size(), format_str,
conditional_conversion(std::forward(args))...);
+ return std::string(buffer.data());
Review comment:
changed them to use either the `(const char*, size_t)` or the `(It
first, It last)` constructors
##
File path: libminifi/include/core/logging/Logger.h
##
@@ -68,13 +69,32 @@ inline T conditional_conversion(T t) {
}
template
-inline std::string format_string(char const* format_str, Args&&... args) {
- char buf[LOG_BUFFER_SIZE];
- std::snprintf(buf, LOG_BUFFER_SIZE, format_str,
conditional_conversion(std::forward(args))...);
- return std::string(buf);
+inline std::string format_string(int max_size, char const* format_str,
Args&&... args) {
+ if (0 <= max_size && max_size <= LOG_BUFFER_SIZE) {
+// use static buffer
+char buf[LOG_BUFFER_SIZE + 1];
+std::snprintf(buf, max_size + 1, format_str,
conditional_conversion(std::forward(args))...);
+return std::string(buf);
+ }
+ // use dynamically allocated buffer
+ if (max_size < 0) {
+// query what buffer size we need
+int size_needed = std::snprintf(nullptr, 0, format_str,
conditional_conversion(std::forward(args))...);
Review comment:
done
--
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
[GitHub] [nifi-minifi-cpp] adamdebreceni commented on a change in pull request #1057: MINIFICPP-1537 - Log heartbeats on demand
adamdebreceni commented on a change in pull request #1057:
URL: https://github.com/apache/nifi-minifi-cpp/pull/1057#discussion_r634577239
##
File path: libminifi/include/c2/HeartbeatJsonSerializer.h
##
@@ -0,0 +1,59 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+#pragma once
+
+#include
+
+#include "HeartbeatReporter.h"
+#include "C2Payload.h"
+
+#ifdef RAPIDJSON_ASSERT
+#undef RAPIDJSON_ASSERT
+#endif
+#define RAPIDJSON_ASSERT(x) if(!(x)) throw std::logic_error("rapidjson
exception"); //NOLINT
+
+#ifdef RAPIDJSON_HAS_STDSTRING
+#undef RAPIDJSON_HAS_STDSTRING
+#endif
+#define RAPIDJSON_HAS_STDSTRING 1
Review comment:
reverted the change, we could change it in a separate PR, updating every
usage
--
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
[GitHub] [nifi-minifi-cpp] adamdebreceni commented on a change in pull request #1057: MINIFICPP-1537 - Log heartbeats on demand
adamdebreceni commented on a change in pull request #1057:
URL: https://github.com/apache/nifi-minifi-cpp/pull/1057#discussion_r634554213
##
File path: libminifi/include/c2/HeartbeatJsonSerializer.h
##
@@ -0,0 +1,59 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+#pragma once
+
+#include
+
+#include "HeartbeatReporter.h"
+#include "C2Payload.h"
+
+#ifdef RAPIDJSON_ASSERT
+#undef RAPIDJSON_ASSERT
+#endif
+#define RAPIDJSON_ASSERT(x) if(!(x)) throw std::logic_error("rapidjson
exception"); //NOLINT
+
+#ifdef RAPIDJSON_HAS_STDSTRING
+#undef RAPIDJSON_HAS_STDSTRING
+#endif
+#define RAPIDJSON_HAS_STDSTRING 1
Review comment:
unclear why it was not used before, enabling it simplifies many json
operations, specifically it gives std::string => rapidjson::Value conversion
--
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
[GitHub] [nifi-minifi-cpp] adamdebreceni commented on a change in pull request #1057: MINIFICPP-1537 - Log heartbeats on demand
adamdebreceni commented on a change in pull request #1057:
URL: https://github.com/apache/nifi-minifi-cpp/pull/1057#discussion_r634554213
##
File path: libminifi/include/c2/HeartbeatJsonSerializer.h
##
@@ -0,0 +1,59 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+#pragma once
+
+#include
+
+#include "HeartbeatReporter.h"
+#include "C2Payload.h"
+
+#ifdef RAPIDJSON_ASSERT
+#undef RAPIDJSON_ASSERT
+#endif
+#define RAPIDJSON_ASSERT(x) if(!(x)) throw std::logic_error("rapidjson
exception"); //NOLINT
+
+#ifdef RAPIDJSON_HAS_STDSTRING
+#undef RAPIDJSON_HAS_STDSTRING
+#endif
+#define RAPIDJSON_HAS_STDSTRING 1
Review comment:
unclear why it was not used before, enabling it simplifies many json
operations
--
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
[GitHub] [nifi-minifi-cpp] adamdebreceni commented on a change in pull request #1057: MINIFICPP-1537 - Log heartbeats on demand
adamdebreceni commented on a change in pull request #1057:
URL: https://github.com/apache/nifi-minifi-cpp/pull/1057#discussion_r634552401
##
File path: libminifi/src/c2/protocols/RESTProtocol.cpp
##
@@ -318,133 +178,7 @@ bool RESTProtocol::containsPayload(const C2Payload ) {
return false;
}
-rapidjson::Value RESTProtocol::serializeConnectionQueues(const C2Payload
, std::string , rapidjson::Document::AllocatorType ) {
- rapidjson::Value json_payload(payload.isContainer() ? rapidjson::kArrayType
: rapidjson::kObjectType);
-
- C2Payload adjusted(payload.getOperation(), payload.getIdentifier(),
payload.isRaw());
-
- auto name = payload.getLabel();
- std::string uuid;
- C2ContentResponse updatedContent(payload.getOperation());
- for (const C2ContentResponse : payload.getContent()) {
-for (const auto& op_arg : content.operation_arguments) {
- if (op_arg.first == "uuid") {
-uuid = op_arg.second.to_string();
- }
- updatedContent.operation_arguments.insert(op_arg);
-}
- }
- updatedContent.name = uuid;
- adjusted.setLabel(uuid);
- adjusted.setIdentifier(uuid);
- c2::AnnotatedValue nd;
- // name should be what was previously the TLN ( top level node )
- nd = name;
- updatedContent.operation_arguments.insert(std::make_pair("name", nd));
- // the rvalue reference is an unfortunate side effect of the underlying API
decision.
- adjusted.addContent(std::move(updatedContent), true);
- mergePayloadContent(json_payload, adjusted, alloc);
- label = uuid;
- return json_payload;
-}
-
-rapidjson::Value RESTProtocol::serializeJsonPayload(const C2Payload ,
rapidjson::Document::AllocatorType ) {
- // get the name from the content
- rapidjson::Value json_payload(payload.isContainer() ? rapidjson::kArrayType
: rapidjson::kObjectType);
-
- std::vector children;
-
- bool isQueue = payload.getLabel() == "queues";
-
- for (const auto _payload : payload.getNestedPayloads()) {
-std::string label = nested_payload.getLabel();
-rapidjson::Value* child_payload = new rapidjson::Value(isQueue ?
serializeConnectionQueues(nested_payload, label, alloc) :
serializeJsonPayload(nested_payload, alloc));
-
-if (nested_payload.isCollapsible()) {
- bool combine = false;
- for (auto : children) {
-if (subordinate.name == label) {
- subordinate.values.push_back(child_payload);
- combine = true;
- break;
-}
- }
- if (!combine) {
-ValueObject obj;
-obj.name = label;
-obj.values.push_back(child_payload);
-children.push_back(obj);
- }
-} else {
- ValueObject obj;
- obj.name = label;
- obj.values.push_back(child_payload);
- children.push_back(obj);
-}
- }
-
- for (auto child_vector : children) {
-rapidjson::Value children_json;
-rapidjson::Value newMemberKey = getStringValue(child_vector.name, alloc);
-if (child_vector.values.size() > 1) {
- children_json.SetArray();
- for (auto child : child_vector.values) {
-if (json_payload.IsArray())
- json_payload.PushBack(child->Move(), alloc);
-else
- children_json.PushBack(child->Move(), alloc);
- }
- if (!json_payload.IsArray())
-json_payload.AddMember(newMemberKey, children_json, alloc);
-} else if (child_vector.values.size() == 1) {
- rapidjson::Value* first = child_vector.values.front();
- if (first->IsObject() && first->HasMember(newMemberKey)) {
-if (json_payload.IsArray())
- json_payload.PushBack((*first)[newMemberKey].Move(), alloc);
-else
- json_payload.AddMember(newMemberKey, (*first)[newMemberKey].Move(),
alloc);
- } else {
-if (json_payload.IsArray()) {
- json_payload.PushBack(first->Move(), alloc);
-} else {
- json_payload.AddMember(newMemberKey, first->Move(), alloc);
-}
- }
-}
-for (rapidjson::Value* child : child_vector.values)
- delete child;
- }
-
- mergePayloadContent(json_payload, payload, alloc);
- return json_payload;
-}
-
-std::string RESTProtocol::getOperation(const C2Payload ) {
- switch (payload.getOperation()) {
-case Operation::ACKNOWLEDGE:
- return "acknowledge";
-case Operation::HEARTBEAT:
- return "heartbeat";
-case Operation::RESTART:
- return "restart";
-case Operation::DESCRIBE:
- return "describe";
-case Operation::STOP:
- return "stop";
-case Operation::START:
- return "start";
-case Operation::UPDATE:
- return "update";
-case Operation::PAUSE:
- return "pause";
-case Operation::RESUME:
- return "resume";
-default:
- return "heartbeat";
- }
-}
-
-Operation RESTProtocol::stringToOperation(const std::string str) {
+Operation RESTProtocol::stringToOperation(const std::string& str) {
Review comment:
made Operation enum into a smart enum, so that string <-> enum
conversions
[GitHub] [nifi-minifi-cpp] adamdebreceni commented on a change in pull request #1057: MINIFICPP-1537 - Log heartbeats on demand
adamdebreceni commented on a change in pull request #1057:
URL: https://github.com/apache/nifi-minifi-cpp/pull/1057#discussion_r634551768
##
File path: libminifi/src/c2/HeartbeatJsonSerializer.cpp
##
@@ -0,0 +1,333 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+#include "c2/HeartbeatJsonSerializer.h"
+#include "rapidjson/document.h"
+#include "rapidjson/writer.h"
+#include "rapidjson/stringbuffer.h"
+#include "rapidjson/prettywriter.h"
+
+namespace org {
+namespace apache {
+namespace nifi {
+namespace minifi {
+namespace c2 {
+
+std::string HeartbeatJsonSerializer::getOperation(const C2Payload& payload) {
+ switch (payload.getOperation()) {
+case Operation::ACKNOWLEDGE:
+ return "acknowledge";
+case Operation::HEARTBEAT:
+ return "heartbeat";
+case Operation::RESTART:
+ return "restart";
+case Operation::DESCRIBE:
+ return "describe";
+case Operation::STOP:
+ return "stop";
+case Operation::START:
+ return "start";
+case Operation::UPDATE:
+ return "update";
+case Operation::PAUSE:
+ return "pause";
+case Operation::RESUME:
+ return "resume";
+default:
+ return "heartbeat";
+ }
Review comment:
unclear why "heartbeat" was used in those instances, those features are
probably not tested, made Operation into a SMART_ENUM with explicit fallback
for stringifying invalid values
--
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
[GitHub] [nifi-minifi-cpp] adamdebreceni commented on a change in pull request #1057: MINIFICPP-1537 - Log heartbeats on demand
adamdebreceni commented on a change in pull request #1057:
URL: https://github.com/apache/nifi-minifi-cpp/pull/1057#discussion_r634550117
##
File path: extensions/http-curl/tests/C2LogHeartbeatTest.cpp
##
@@ -0,0 +1,58 @@
+/**
+ *
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+#undef NDEBUG
+#include "TestBase.h"
+#include "c2/C2Agent.h"
+#include "protocols/RESTProtocol.h"
+#include "protocols/RESTSender.h"
+#include "HTTPIntegrationBase.h"
+#include "HTTPHandlers.h"
+#include "utils/IntegrationTestUtils.h"
+#include "c2/HeartbeatLogger.h"
+
+class VerifyLogC2Heartbeat : public VerifyC2Base {
+ public:
+ void testSetup() override {
+LogTestController::getInstance().setTrace();
+LogTestController::getInstance().setDebug();
+LogTestController::getInstance().setDebug();
+// the heartbeat is logged at TRACE level
+LogTestController::getInstance().setTrace();
+VerifyC2Base::testSetup();
+ }
+
+ void runAssertions() override {
+using org::apache::nifi::minifi::utils::verifyLogLinePresenceInPollTime;
+assert(verifyLogLinePresenceInPollTime(
+std::chrono::milliseconds(wait_time_),
+"\"operation\": \"heartbeat\""));
+ }
+
+ void configureC2() override {
+VerifyC2Base::configureC2();
+configuration->set("c2.agent.heartbeat.reporter.classes",
"HeartbeatLogger");
Review comment:
done
--
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
[GitHub] [nifi-minifi-cpp] adamdebreceni commented on a change in pull request #1057: MINIFICPP-1537 - Log heartbeats on demand
adamdebreceni commented on a change in pull request #1057: URL: https://github.com/apache/nifi-minifi-cpp/pull/1057#discussion_r630823120 ## File path: conf/minifi.properties ## @@ -67,6 +67,7 @@ nifi.content.repository.class.name=DatabaseContentRepository ## define protocol parameters ## The default is CoAP, if that extension is built. ## Alternatively, you may use RESTSender if http-curl is built +#nifi.c2.agent.protocol.class=RESTSender Review comment: it became obsolete in light of #1058, removed -- 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
