Re: [PR] MINIFICPP-2262 Add processor to push logs to Grafana Loki through gRPC [nifi-minifi-cpp]
szaszm closed pull request #1698: MINIFICPP-2262 Add processor to push logs to Grafana Loki through gRPC URL: https://github.com/apache/nifi-minifi-cpp/pull/1698 -- 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: issues-unsubscr...@nifi.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] MINIFICPP-2262 Add processor to push logs to Grafana Loki through gRPC [nifi-minifi-cpp]
lordgamez commented on code in PR #1698:
URL: https://github.com/apache/nifi-minifi-cpp/pull/1698#discussion_r1453461616
##
extensions/grafana-loki/PushGrafanaLoki.cpp:
##
@@ -0,0 +1,249 @@
+/**
+ * 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 "PushGrafanaLoki.h"
+
+#include
+
+#include "core/ProcessContext.h"
+#include "core/ProcessSession.h"
+#include "utils/ProcessorConfigUtils.h"
+#include "utils/StringUtils.h"
+
+namespace org::apache::nifi::minifi::extensions::grafana::loki {
+
+void PushGrafanaLoki::LogBatch::add(const std::shared_ptr&
flowfile) {
+ gsl_Expects(state_manager_);
+ if (log_line_batch_wait_ && batched_flowfiles_.empty()) {
+start_push_time_ = std::chrono::steady_clock::now();
+std::unordered_map state;
+state["start_push_time"] =
std::to_string(std::chrono::duration_cast(start_push_time_.time_since_epoch()).count());
+logger_->log_debug("Saved start push time to state: {}",
state["start_push_time"]);
+state_manager_->set(state);
+ }
+ batched_flowfiles_.push_back(flowfile);
+}
+
+void PushGrafanaLoki::LogBatch::restore(const std::shared_ptr&
flowfile) {
+ batched_flowfiles_.push_back(flowfile);
+}
+
+std::vector>
PushGrafanaLoki::LogBatch::flush() {
+ gsl_Expects(state_manager_);
+ start_push_time_ = {};
+ auto result = batched_flowfiles_;
+ batched_flowfiles_.clear();
+ if (log_line_batch_wait_) {
+start_push_time_ = {};
+std::unordered_map state;
+logger_->log_debug("Reset start push time state");
+state["start_push_time"] = "0";
+state_manager_->set(state);
+ }
+ return result;
+}
+
+bool PushGrafanaLoki::LogBatch::isReady() const {
+ return (log_line_batch_size_ && batched_flowfiles_.size() >=
*log_line_batch_size_) || (log_line_batch_wait_ &&
std::chrono::steady_clock::now() - start_push_time_ >= *log_line_batch_wait_);
+}
+
+void PushGrafanaLoki::LogBatch::setLogLineBatchSize(std::optional
log_line_batch_size) {
+ log_line_batch_size_ = log_line_batch_size;
+}
+
+void
PushGrafanaLoki::LogBatch::setLogLineBatchWait(std::optional
log_line_batch_wait) {
+ log_line_batch_wait_ = log_line_batch_wait;
+}
+
+void PushGrafanaLoki::LogBatch::setStateManager(core::StateManager*
state_manager) {
+ state_manager_ = state_manager;
+}
+
+void
PushGrafanaLoki::LogBatch::setStartPushTime(std::chrono::steady_clock::time_point
start_push_time) {
+ start_push_time_ = start_push_time;
+}
+
+const core::Relationship PushGrafanaLoki::Self("__self__", "Marks the FlowFile
to be owned by this processor");
+
+std::shared_ptr
PushGrafanaLoki::getSSLContextService(core::ProcessContext& context) {
+ if (auto ssl_context =
context.getProperty(PushGrafanaLoki::SSLContextService)) {
+return
std::dynamic_pointer_cast(context.getControllerService(*ssl_context));
+ }
+ return std::shared_ptr{};
+}
+
+std::string PushGrafanaLoki::readLogLineFromFlowFile(const
std::shared_ptr& flow_file, core::ProcessSession& session) {
+ auto read_buffer_result = session.readBuffer(flow_file);
+ return {reinterpret_cast(read_buffer_result.buffer.data()),
read_buffer_result.buffer.size()};
+}
+
+void PushGrafanaLoki::setUpStateManager(core::ProcessContext& context) {
+ auto state_manager = context.getStateManager();
+ if (state_manager == nullptr) {
+throw Exception(PROCESSOR_EXCEPTION, "Failed to get StateManager");
+ }
+ log_batch_.setStateManager(state_manager);
+
+ std::unordered_map state_map;
+ if (state_manager->get(state_map)) {
+auto it = state_map.find("start_push_time");
+if (it != state_map.end()) {
+ logger_->log_info("Restored start push time from processor state: {}",
it->second);
+ std::chrono::steady_clock::time_point
start_push_time{std::chrono::milliseconds{std::stoll(it->second)}};
+ log_batch_.setStartPushTime(start_push_time);
+}
+ }
+}
+
+std::map
PushGrafanaLoki::buildStreamLabelMap(core::ProcessContext& context) {
+ std::map stream_label_map;
+ if (auto stream_labels_str = context.getProperty(StreamLabels)) {
+auto stream_labels =
utils::string::splitAndTrimRemovingEmpty(*stream_labels_str, ",");
+if (stream_labels.empty()) {
+ throw Exception(PROCESS_SCHEDULE_EXCEPTION, "Missing or inval
Re: [PR] MINIFICPP-2262 Add processor to push logs to Grafana Loki through gRPC [nifi-minifi-cpp]
szaszm commented on code in PR #1698:
URL: https://github.com/apache/nifi-minifi-cpp/pull/1698#discussion_r1453403835
##
extensions/grafana-loki/PushGrafanaLoki.cpp:
##
@@ -0,0 +1,249 @@
+/**
+ * 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 "PushGrafanaLoki.h"
+
+#include
+
+#include "core/ProcessContext.h"
+#include "core/ProcessSession.h"
+#include "utils/ProcessorConfigUtils.h"
+#include "utils/StringUtils.h"
+
+namespace org::apache::nifi::minifi::extensions::grafana::loki {
+
+void PushGrafanaLoki::LogBatch::add(const std::shared_ptr&
flowfile) {
+ gsl_Expects(state_manager_);
+ if (log_line_batch_wait_ && batched_flowfiles_.empty()) {
+start_push_time_ = std::chrono::steady_clock::now();
+std::unordered_map state;
+state["start_push_time"] =
std::to_string(std::chrono::duration_cast(start_push_time_.time_since_epoch()).count());
+logger_->log_debug("Saved start push time to state: {}",
state["start_push_time"]);
+state_manager_->set(state);
+ }
+ batched_flowfiles_.push_back(flowfile);
+}
+
+void PushGrafanaLoki::LogBatch::restore(const std::shared_ptr&
flowfile) {
+ batched_flowfiles_.push_back(flowfile);
+}
+
+std::vector>
PushGrafanaLoki::LogBatch::flush() {
+ gsl_Expects(state_manager_);
+ start_push_time_ = {};
+ auto result = batched_flowfiles_;
+ batched_flowfiles_.clear();
+ if (log_line_batch_wait_) {
+start_push_time_ = {};
+std::unordered_map state;
+logger_->log_debug("Reset start push time state");
+state["start_push_time"] = "0";
+state_manager_->set(state);
+ }
+ return result;
+}
+
+bool PushGrafanaLoki::LogBatch::isReady() const {
+ return (log_line_batch_size_ && batched_flowfiles_.size() >=
*log_line_batch_size_) || (log_line_batch_wait_ &&
std::chrono::steady_clock::now() - start_push_time_ >= *log_line_batch_wait_);
+}
+
+void PushGrafanaLoki::LogBatch::setLogLineBatchSize(std::optional
log_line_batch_size) {
+ log_line_batch_size_ = log_line_batch_size;
+}
+
+void
PushGrafanaLoki::LogBatch::setLogLineBatchWait(std::optional
log_line_batch_wait) {
+ log_line_batch_wait_ = log_line_batch_wait;
+}
+
+void PushGrafanaLoki::LogBatch::setStateManager(core::StateManager*
state_manager) {
+ state_manager_ = state_manager;
+}
+
+void
PushGrafanaLoki::LogBatch::setStartPushTime(std::chrono::steady_clock::time_point
start_push_time) {
+ start_push_time_ = start_push_time;
+}
+
+const core::Relationship PushGrafanaLoki::Self("__self__", "Marks the FlowFile
to be owned by this processor");
+
+std::shared_ptr
PushGrafanaLoki::getSSLContextService(core::ProcessContext& context) {
+ if (auto ssl_context =
context.getProperty(PushGrafanaLoki::SSLContextService)) {
+return
std::dynamic_pointer_cast(context.getControllerService(*ssl_context));
+ }
+ return std::shared_ptr{};
+}
+
+std::string PushGrafanaLoki::readLogLineFromFlowFile(const
std::shared_ptr& flow_file, core::ProcessSession& session) {
+ auto read_buffer_result = session.readBuffer(flow_file);
+ return {reinterpret_cast(read_buffer_result.buffer.data()),
read_buffer_result.buffer.size()};
+}
+
+void PushGrafanaLoki::setUpStateManager(core::ProcessContext& context) {
+ auto state_manager = context.getStateManager();
+ if (state_manager == nullptr) {
+throw Exception(PROCESSOR_EXCEPTION, "Failed to get StateManager");
+ }
+ log_batch_.setStateManager(state_manager);
+
+ std::unordered_map state_map;
+ if (state_manager->get(state_map)) {
+auto it = state_map.find("start_push_time");
+if (it != state_map.end()) {
+ logger_->log_info("Restored start push time from processor state: {}",
it->second);
+ std::chrono::steady_clock::time_point
start_push_time{std::chrono::milliseconds{std::stoll(it->second)}};
+ log_batch_.setStartPushTime(start_push_time);
+}
+ }
+}
+
+std::map
PushGrafanaLoki::buildStreamLabelMap(core::ProcessContext& context) {
+ std::map stream_label_map;
+ if (auto stream_labels_str = context.getProperty(StreamLabels)) {
+auto stream_labels =
utils::string::splitAndTrimRemovingEmpty(*stream_labels_str, ",");
+if (stream_labels.empty()) {
+ throw Exception(PROCESS_SCHEDULE_EXCEPTION, "Missing or invalid
Re: [PR] MINIFICPP-2262 Add processor to push logs to Grafana Loki through gRPC [nifi-minifi-cpp]
lordgamez commented on code in PR #1698: URL: https://github.com/apache/nifi-minifi-cpp/pull/1698#discussion_r1452556325 ## extensions/grafana-loki/protos/grafana-loki-push.proto: ## Review Comment: Added reference in 407abbc65a8900dc9937dc0587c52d68693335ff -- 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: issues-unsubscr...@nifi.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] MINIFICPP-2262 Add processor to push logs to Grafana Loki through gRPC [nifi-minifi-cpp]
lordgamez commented on code in PR #1698: URL: https://github.com/apache/nifi-minifi-cpp/pull/1698#discussion_r1452556096 ## .dockerignore: ## @@ -57,6 +57,11 @@ extensions/expression-language/Scanner.cpp extensions/expression-language/location.hh extensions/expression-language/position.hh extensions/expression-language/stack.h +extensions/grafana-loki/protobuf-generated/grafana-loki-push.grpc.pb.h +extensions/grafana-loki/protobuf-generated/grafana-loki-push.grpc.pb.cc +extensions/grafana-loki/protobuf-generated/grafana-loki-push.pb.h +extensions/grafana-loki/protobuf-generated/grafana-loki-push.pb.cc Review Comment: Good idea, moved them in 407abbc65a8900dc9937dc0587c52d68693335ff -- 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: issues-unsubscr...@nifi.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] MINIFICPP-2262 Add processor to push logs to Grafana Loki through gRPC [nifi-minifi-cpp]
lordgamez commented on code in PR #1698:
URL: https://github.com/apache/nifi-minifi-cpp/pull/1698#discussion_r1452555861
##
extensions/grafana-loki/CMakeLists.txt:
##
@@ -16,20 +16,46 @@
# specific language governing permissions and limitations
# under the License.
#
-
if (NOT (ENABLE_ALL OR ENABLE_GRAFANA_LOKI))
return()
endif()
include(${CMAKE_SOURCE_DIR}/extensions/ExtensionHeader.txt)
-file(GLOB SOURCES "*.cpp")
+if (ENABLE_GRPC)
+include(Grpc)
-add_library(minifi-grafana-loki SHARED ${SOURCES})
+file(MAKE_DIRECTORY ${CMAKE_CURRENT_SOURCE_DIR}/protobuf-generated)
+
+add_custom_command(
+OUTPUT
${CMAKE_CURRENT_SOURCE_DIR}/protobuf-generated/grafana-loki-push.grpc.pb.cc
${CMAKE_CURRENT_SOURCE_DIR}/protobuf-generated/grafana-loki-push.grpc.pb.h
${CMAKE_CURRENT_SOURCE_DIR}/protobuf-generated/grafana-loki-push.pb.h
${CMAKE_CURRENT_SOURCE_DIR}/protobuf-generated/grafana-loki-push.pb.cc
+COMMAND ${PROTOBUF_COMPILER}
--plugin=protoc-gen-grpc=${GRPC_CPP_PLUGIN}
-I=${CMAKE_CURRENT_SOURCE_DIR}/protos/ -I=${protobuf_SOURCE_DIR}/src
--grpc_out=${CMAKE_CURRENT_SOURCE_DIR}/protobuf-generated/
--cpp_out=${CMAKE_CURRENT_SOURCE_DIR}/protobuf-generated/
${CMAKE_CURRENT_SOURCE_DIR}/protos/grafana-loki-push.proto
+DEPENDS protobuf::protoc grpc_cpp_plugin)
+add_custom_target(grafana-loki-protos ALL DEPENDS
${CMAKE_CURRENT_SOURCE_DIR}/protobuf-generated/grafana-loki-push.grpc.pb.cc
${CMAKE_CURRENT_SOURCE_DIR}/protobuf-generated/grafana-loki-push.grpc.pb.h
${CMAKE_CURRENT_SOURCE_DIR}/protobuf-generated/grafana-loki-push.pb.h
${CMAKE_CURRENT_SOURCE_DIR}/protobuf-generated/grafana-loki-push.pb.cc)
+
+file(GLOB SOURCES "*.cpp")
+list(APPEND SOURCES
+
${CMAKE_CURRENT_SOURCE_DIR}/protobuf-generated/grafana-loki-push.grpc.pb.cc
+${CMAKE_CURRENT_SOURCE_DIR}/protobuf-generated/grafana-loki-push.pb.cc
+)
+else()
+set(SOURCES
+${CMAKE_CURRENT_SOURCE_DIR}/PushGrafanaLoki.cpp
+${CMAKE_CURRENT_SOURCE_DIR}/PushGrafanaLokiREST.cpp
+)
+endif()
+
+add_library(minifi-grafana-loki SHARED ${SOURCES})
target_include_directories(minifi-grafana-loki PRIVATE BEFORE
"${CMAKE_SOURCE_DIR}/extensions/http-curl")
-target_link_libraries(minifi-grafana-loki ${LIBMINIFI})
-target_link_libraries(minifi-grafana-loki minifi-http-curl)
+target_link_libraries(minifi-grafana-loki ${LIBMINIFI} minifi-http-curl)
+add_dependencies(minifi-grafana-loki minifi-http-curl)
+
+if (ENABLE_GRPC)
+target_include_directories(minifi-grafana-loki PRIVATE BEFORE
"${GRPC_INCLUDE_DIR}" "${PROTOBUF_INCLUDE_DIR}")
+target_link_libraries(minifi-grafana-loki grpc++ protobuf::libprotobuf)
+add_dependencies(minifi-grafana-loki grpc grpc++ protobuf::libprotobuf
grafana-loki-protos)
Review Comment:
The grafana-loki-protos is the target to generate the protobuf cpp files,
the minifi-grafana-loki target should depend on that, but you are right the
other targets should not be included in the dependencies, updated in
407abbc65a8900dc9937dc0587c52d68693335ff
--
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: issues-unsubscr...@nifi.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
Re: [PR] MINIFICPP-2262 Add processor to push logs to Grafana Loki through gRPC [nifi-minifi-cpp]
lordgamez commented on code in PR #1698: URL: https://github.com/apache/nifi-minifi-cpp/pull/1698#discussion_r1452554318 ## cmake/MiNiFiOptions.cmake: ## @@ -122,6 +122,7 @@ add_minifi_option(ENABLE_KUBERNETES "Enables the Kubernetes extensions." ON) add_minifi_option(ENABLE_TEST_PROCESSORS "Enables test processors" OFF) add_minifi_option(ENABLE_PROMETHEUS "Enables Prometheus support." ON) add_minifi_option(ENABLE_GRAFANA_LOKI "Enable Grafana Loki support" OFF) +add_minifi_option(ENABLE_GRPC "Enable gRPC for Grafana Loki extension" ON) Review Comment: Good point, renamed to `ENABLE_GRPC_FOR_LOKI` in 407abbc65a8900dc9937dc0587c52d68693335ff -- 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: issues-unsubscr...@nifi.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] MINIFICPP-2262 Add processor to push logs to Grafana Loki through gRPC [nifi-minifi-cpp]
lordgamez commented on code in PR #1698: URL: https://github.com/apache/nifi-minifi-cpp/pull/1698#discussion_r1452482580 ## LICENSE: ## @@ -2442,52 +2443,6 @@ THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. -This product bundles 'protobuf' within 'OpenCV' under a 3-Clause BSD license: Review Comment: Yes, I disabled the build of the dnn module of OpenCV in `cmake/BundledOpenCV.cmake` that used the protobuf library as we don't use it in our OpenCV processors. -- 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: issues-unsubscr...@nifi.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] MINIFICPP-2262 Add processor to push logs to Grafana Loki through gRPC [nifi-minifi-cpp]
lordgamez commented on code in PR #1698: URL: https://github.com/apache/nifi-minifi-cpp/pull/1698#discussion_r1452480781 ## extensions/grafana-loki/protos/grafana-loki-push.proto: ## Review Comment: This file defines the grpc service and the protobuf messages structures according to grafana's protocol definition in this file: https://github.com/grafana/loki/blob/main/pkg/push/push.proto Only the used fields and attributes are kept, removing the go specific attributes and imports. In the Grafana repository the containing directory holds an Apache license (https://github.com/grafana/loki/blob/main/pkg/push/LICENSE) so I think we should be okay with the license header, I can add a reference comment to the original proto file it that's okay. -- 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: issues-unsubscr...@nifi.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] MINIFICPP-2262 Add processor to push logs to Grafana Loki through gRPC [nifi-minifi-cpp]
szaszm commented on code in PR #1698: URL: https://github.com/apache/nifi-minifi-cpp/pull/1698#discussion_r1452235982 ## extensions/grafana-loki/protos/grafana-loki-push.proto: ## Review Comment: Is this file coming from somewhere else? We should include the source, and use the appropriate license. -- 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: issues-unsubscr...@nifi.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] MINIFICPP-2262 Add processor to push logs to Grafana Loki through gRPC [nifi-minifi-cpp]
szaszm commented on code in PR #1698:
URL: https://github.com/apache/nifi-minifi-cpp/pull/1698#discussion_r1452228611
##
extensions/grafana-loki/CMakeLists.txt:
##
@@ -16,20 +16,46 @@
# specific language governing permissions and limitations
# under the License.
#
-
if (NOT (ENABLE_ALL OR ENABLE_GRAFANA_LOKI))
return()
endif()
include(${CMAKE_SOURCE_DIR}/extensions/ExtensionHeader.txt)
-file(GLOB SOURCES "*.cpp")
+if (ENABLE_GRPC)
+include(Grpc)
-add_library(minifi-grafana-loki SHARED ${SOURCES})
+file(MAKE_DIRECTORY ${CMAKE_CURRENT_SOURCE_DIR}/protobuf-generated)
+
+add_custom_command(
+OUTPUT
${CMAKE_CURRENT_SOURCE_DIR}/protobuf-generated/grafana-loki-push.grpc.pb.cc
${CMAKE_CURRENT_SOURCE_DIR}/protobuf-generated/grafana-loki-push.grpc.pb.h
${CMAKE_CURRENT_SOURCE_DIR}/protobuf-generated/grafana-loki-push.pb.h
${CMAKE_CURRENT_SOURCE_DIR}/protobuf-generated/grafana-loki-push.pb.cc
+COMMAND ${PROTOBUF_COMPILER}
--plugin=protoc-gen-grpc=${GRPC_CPP_PLUGIN}
-I=${CMAKE_CURRENT_SOURCE_DIR}/protos/ -I=${protobuf_SOURCE_DIR}/src
--grpc_out=${CMAKE_CURRENT_SOURCE_DIR}/protobuf-generated/
--cpp_out=${CMAKE_CURRENT_SOURCE_DIR}/protobuf-generated/
${CMAKE_CURRENT_SOURCE_DIR}/protos/grafana-loki-push.proto
+DEPENDS protobuf::protoc grpc_cpp_plugin)
+add_custom_target(grafana-loki-protos ALL DEPENDS
${CMAKE_CURRENT_SOURCE_DIR}/protobuf-generated/grafana-loki-push.grpc.pb.cc
${CMAKE_CURRENT_SOURCE_DIR}/protobuf-generated/grafana-loki-push.grpc.pb.h
${CMAKE_CURRENT_SOURCE_DIR}/protobuf-generated/grafana-loki-push.pb.h
${CMAKE_CURRENT_SOURCE_DIR}/protobuf-generated/grafana-loki-push.pb.cc)
+
+file(GLOB SOURCES "*.cpp")
+list(APPEND SOURCES
+
${CMAKE_CURRENT_SOURCE_DIR}/protobuf-generated/grafana-loki-push.grpc.pb.cc
+${CMAKE_CURRENT_SOURCE_DIR}/protobuf-generated/grafana-loki-push.pb.cc
+)
+else()
+set(SOURCES
+${CMAKE_CURRENT_SOURCE_DIR}/PushGrafanaLoki.cpp
+${CMAKE_CURRENT_SOURCE_DIR}/PushGrafanaLokiREST.cpp
+)
+endif()
+
+add_library(minifi-grafana-loki SHARED ${SOURCES})
target_include_directories(minifi-grafana-loki PRIVATE BEFORE
"${CMAKE_SOURCE_DIR}/extensions/http-curl")
-target_link_libraries(minifi-grafana-loki ${LIBMINIFI})
-target_link_libraries(minifi-grafana-loki minifi-http-curl)
+target_link_libraries(minifi-grafana-loki ${LIBMINIFI} minifi-http-curl)
+add_dependencies(minifi-grafana-loki minifi-http-curl)
+
+if (ENABLE_GRPC)
+target_include_directories(minifi-grafana-loki PRIVATE BEFORE
"${GRPC_INCLUDE_DIR}" "${PROTOBUF_INCLUDE_DIR}")
+target_link_libraries(minifi-grafana-loki grpc++ protobuf::libprotobuf)
+add_dependencies(minifi-grafana-loki grpc grpc++ protobuf::libprotobuf
grafana-loki-protos)
Review Comment:
`target_link_libraries`: they should be linked together
`add_dependencies`: these targets should be compiled before the first in the
list, not allowing concurrent compilation.
We should avoid `add_dependencies`, unless we're running the binaries of the
result of a previous build step, or similar. In this case, is that the
generation of sources with protobuf?
--
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: issues-unsubscr...@nifi.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
Re: [PR] MINIFICPP-2262 Add processor to push logs to Grafana Loki through gRPC [nifi-minifi-cpp]
szaszm commented on code in PR #1698: URL: https://github.com/apache/nifi-minifi-cpp/pull/1698#discussion_r1446385712 ## LICENSE: ## @@ -2442,52 +2443,6 @@ THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. -This product bundles 'protobuf' within 'OpenCV' under a 3-Clause BSD license: Review Comment: Is it no longer enabled within OpenCV? ## .dockerignore: ## @@ -57,6 +57,11 @@ extensions/expression-language/Scanner.cpp extensions/expression-language/location.hh extensions/expression-language/position.hh extensions/expression-language/stack.h +extensions/grafana-loki/protobuf-generated/grafana-loki-push.grpc.pb.h +extensions/grafana-loki/protobuf-generated/grafana-loki-push.grpc.pb.cc +extensions/grafana-loki/protobuf-generated/grafana-loki-push.pb.h +extensions/grafana-loki/protobuf-generated/grafana-loki-push.pb.cc Review Comment: Could we generate these sources in the build directory instead, and add them to the build from there? -- 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: issues-unsubscr...@nifi.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] MINIFICPP-2262 Add processor to push logs to Grafana Loki through gRPC [nifi-minifi-cpp]
adamdebreceni commented on code in PR #1698:
URL: https://github.com/apache/nifi-minifi-cpp/pull/1698#discussion_r1447307752
##
extensions/grafana-loki/CMakeLists.txt:
##
@@ -16,20 +16,46 @@
# specific language governing permissions and limitations
# under the License.
#
-
if (NOT (ENABLE_ALL OR ENABLE_GRAFANA_LOKI))
return()
endif()
include(${CMAKE_SOURCE_DIR}/extensions/ExtensionHeader.txt)
-file(GLOB SOURCES "*.cpp")
+if (ENABLE_GRPC)
+include(Grpc)
-add_library(minifi-grafana-loki SHARED ${SOURCES})
+file(MAKE_DIRECTORY ${CMAKE_CURRENT_SOURCE_DIR}/protobuf-generated)
+
+add_custom_command(
+OUTPUT
${CMAKE_CURRENT_SOURCE_DIR}/protobuf-generated/grafana-loki-push.grpc.pb.cc
${CMAKE_CURRENT_SOURCE_DIR}/protobuf-generated/grafana-loki-push.grpc.pb.h
${CMAKE_CURRENT_SOURCE_DIR}/protobuf-generated/grafana-loki-push.pb.h
${CMAKE_CURRENT_SOURCE_DIR}/protobuf-generated/grafana-loki-push.pb.cc
+COMMAND ${PROTOBUF_COMPILER}
--plugin=protoc-gen-grpc=${GRPC_CPP_PLUGIN}
-I=${CMAKE_CURRENT_SOURCE_DIR}/protos/ -I=${protobuf_SOURCE_DIR}/src
--grpc_out=${CMAKE_CURRENT_SOURCE_DIR}/protobuf-generated/
--cpp_out=${CMAKE_CURRENT_SOURCE_DIR}/protobuf-generated/
${CMAKE_CURRENT_SOURCE_DIR}/protos/grafana-loki-push.proto
+DEPENDS protobuf::protoc grpc_cpp_plugin)
+add_custom_target(grafana-loki-protos ALL DEPENDS
${CMAKE_CURRENT_SOURCE_DIR}/protobuf-generated/grafana-loki-push.grpc.pb.cc
${CMAKE_CURRENT_SOURCE_DIR}/protobuf-generated/grafana-loki-push.grpc.pb.h
${CMAKE_CURRENT_SOURCE_DIR}/protobuf-generated/grafana-loki-push.pb.h
${CMAKE_CURRENT_SOURCE_DIR}/protobuf-generated/grafana-loki-push.pb.cc)
+
+file(GLOB SOURCES "*.cpp")
+list(APPEND SOURCES
+
${CMAKE_CURRENT_SOURCE_DIR}/protobuf-generated/grafana-loki-push.grpc.pb.cc
+${CMAKE_CURRENT_SOURCE_DIR}/protobuf-generated/grafana-loki-push.pb.cc
+)
+else()
+set(SOURCES
+${CMAKE_CURRENT_SOURCE_DIR}/PushGrafanaLoki.cpp
+${CMAKE_CURRENT_SOURCE_DIR}/PushGrafanaLokiREST.cpp
+)
+endif()
+
+add_library(minifi-grafana-loki SHARED ${SOURCES})
target_include_directories(minifi-grafana-loki PRIVATE BEFORE
"${CMAKE_SOURCE_DIR}/extensions/http-curl")
-target_link_libraries(minifi-grafana-loki ${LIBMINIFI})
-target_link_libraries(minifi-grafana-loki minifi-http-curl)
+target_link_libraries(minifi-grafana-loki ${LIBMINIFI} minifi-http-curl)
+add_dependencies(minifi-grafana-loki minifi-http-curl)
+
+if (ENABLE_GRPC)
+target_include_directories(minifi-grafana-loki PRIVATE BEFORE
"${GRPC_INCLUDE_DIR}" "${PROTOBUF_INCLUDE_DIR}")
+target_link_libraries(minifi-grafana-loki grpc++ protobuf::libprotobuf)
+add_dependencies(minifi-grafana-loki grpc grpc++ protobuf::libprotobuf
grafana-loki-protos)
Review Comment:
is the `grpc++` and `protobuf::libprotobuf` dependency already handled by
`target_link_libraries`?
--
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: issues-unsubscr...@nifi.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
Re: [PR] MINIFICPP-2262 Add processor to push logs to Grafana Loki through gRPC [nifi-minifi-cpp]
martinzink commented on code in PR #1698:
URL: https://github.com/apache/nifi-minifi-cpp/pull/1698#discussion_r1418662196
##
cmake/MiNiFiOptions.cmake:
##
@@ -122,6 +122,7 @@ add_minifi_option(ENABLE_KUBERNETES "Enables the Kubernetes
extensions." ON)
add_minifi_option(ENABLE_TEST_PROCESSORS "Enables test processors" OFF)
add_minifi_option(ENABLE_PROMETHEUS "Enables Prometheus support." ON)
add_minifi_option(ENABLE_GRAFANA_LOKI "Enable Grafana Loki support" OFF)
+add_minifi_option(ENABLE_GRPC "Enable gRPC for Grafana Loki extension" ON)
Review Comment:
ENABLE_GRPC seems a bit too generic since GRPC can be used for a lot of
other things.
##
CMakeLists.txt:
##
@@ -352,6 +356,10 @@ if (ENABLE_ALL OR ENABLE_PROMETHEUS OR NOT DISABLE_CIVET)
list(APPEND CMAKE_MODULE_PATH "${CMAKE_SOURCE_DIR}/cmake/civetweb/dummy")
endif()
+if (ENABLE_ALL OR ENABLE_GCP OR ENABLE_GRAFANA_LOKI)
+include(Abseil)
+endif()
Review Comment:
Why do we need this here? Couldnt we include abseil twice? in gcp and
grafana aswell (after the early returns ofcourse), since its a fetchcontent
makeavailable we should be able to call it twice
--
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: issues-unsubscr...@nifi.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
[PR] MINIFICPP-2262 Add processor to push logs to Grafana Loki through gRPC [nifi-minifi-cpp]
lordgamez opened a new pull request, #1698: URL: https://github.com/apache/nifi-minifi-cpp/pull/1698 https://issues.apache.org/jira/browse/MINIFICPP-2262 --- Thank you for submitting a contribution to Apache NiFi - MiNiFi C++. In order to streamline the review of the contribution we ask you to ensure the following steps have been taken: ### For all changes: - [ ] Is there a JIRA ticket associated with this PR? Is it referenced in the commit message? - [ ] Does your PR title start with MINIFICPP- where is the JIRA number you are trying to resolve? Pay particular attention to the hyphen "-" character. - [ ] Has your PR been rebased against the latest commit within the target branch (typically main)? - [ ] Is your initial contribution a single, squashed commit? ### For code changes: - [ ] If adding new dependencies to the code, are these dependencies licensed in a way that is compatible for inclusion under [ASF 2.0](http://www.apache.org/legal/resolved.html#category-a)? - [ ] If applicable, have you updated the LICENSE file? - [ ] If applicable, have you updated the NOTICE file? ### For documentation related changes: - [ ] Have you ensured that format looks appropriate for the output in which it is rendered? ### Note: Please ensure that once the PR is submitted, you check GitHub Actions CI results for build issues and submit an update to your PR as soon as possible. -- 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: issues-unsubscr...@nifi.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
