You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@nifi.apache.org by GitBox <gi...@apache.org> on 2022/12/15 17:20:24 UTC
[GitHub] [nifi-minifi-cpp] fgerlits opened a new pull request, #1481: MINIFICPP-1948 Add the UUID to the end of Processor and Controller Service log lines
fgerlits opened a new pull request, #1481:
URL: https://github.com/apache/nifi-minifi-cpp/pull/1481
https://issues.apache.org/jira/browse/MINIFICPP-1948
Add the component UUID to the end of lines logged by Processors and Controller Services. See an example in the Jira.
The feature is on by default, but can be turned off in `minifi-log.properties`.
---
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:
- [x] Is there a JIRA ticket associated with this PR? Is it referenced
in the commit message?
- [x] Does your PR title start with MINIFICPP-XXXX where XXXX is the JIRA number you are trying to resolve? Pay particular attention to the hyphen "-" character.
- [x] 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-unsubscribe@nifi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [nifi-minifi-cpp] szaszm commented on a diff in pull request #1481: MINIFICPP-1948 Add the UUID to the end of Processor and Controller Service log lines
Posted by GitBox <gi...@apache.org>.
szaszm commented on code in PR #1481:
URL: https://github.com/apache/nifi-minifi-cpp/pull/1481#discussion_r1053362619
##########
libminifi/include/core/logging/LoggerConfiguration.h:
##########
@@ -133,16 +132,21 @@ class LoggerConfiguration {
class LoggerImpl : public Logger {
public:
- explicit LoggerImpl(std::string name, const std::shared_ptr<LoggerControl> &controller, const std::shared_ptr<spdlog::logger> &delegate)
+ explicit LoggerImpl(std::string name, std::optional<std::string> id, const std::shared_ptr<LoggerControl> &controller, const std::shared_ptr<spdlog::logger> &delegate)
Review Comment:
Do you think we can pass an `Identifier` instead? It's cheaper to copy, and we can still format it appropriately before storing it in the string member.
--
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-unsubscribe@nifi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [nifi-minifi-cpp] fgerlits commented on a diff in pull request #1481: MINIFICPP-1948 Add the UUID to the end of Processor and Controller Service log lines
Posted by GitBox <gi...@apache.org>.
fgerlits commented on code in PR #1481:
URL: https://github.com/apache/nifi-minifi-cpp/pull/1481#discussion_r1055622646
##########
libminifi/include/core/logging/LoggerConfiguration.h:
##########
@@ -133,16 +132,21 @@ class LoggerConfiguration {
class LoggerImpl : public Logger {
public:
- explicit LoggerImpl(std::string name, const std::shared_ptr<LoggerControl> &controller, const std::shared_ptr<spdlog::logger> &delegate)
+ explicit LoggerImpl(std::string name, std::optional<std::string> id, const std::shared_ptr<LoggerControl> &controller, const std::shared_ptr<spdlog::logger> &delegate)
Review Comment:
I have changed everything to `Identifier` in 8e2a3e25602436e36b686114ea7ad4daef5f6be8
--
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-unsubscribe@nifi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [nifi-minifi-cpp] lordgamez closed pull request #1481: MINIFICPP-1948 Add the UUID to the end of Processor and Controller Service log lines
Posted by GitBox <gi...@apache.org>.
lordgamez closed pull request #1481: MINIFICPP-1948 Add the UUID to the end of Processor and Controller Service log lines
URL: https://github.com/apache/nifi-minifi-cpp/pull/1481
--
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-unsubscribe@nifi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [nifi-minifi-cpp] martinzink commented on pull request #1481: MINIFICPP-1948 Add the UUID to the end of Processor and Controller Service log lines
Posted by GitBox <gi...@apache.org>.
martinzink commented on PR #1481:
URL: https://github.com/apache/nifi-minifi-cpp/pull/1481#issuecomment-1355165904
> > I've found some constructors where the passing of the uuid is still missing
>
> Thanks! I have added most of these, including tensorflow, in [19a655b](https://github.com/apache/nifi-minifi-cpp/commit/19a655bb6b4fae6feb9bf897d5dc327f09e4225b).
>
> I did not add UUID to the loggers of `ThreadManagementService`, `StandardControllerServiceNode` and `StandardControllerServiceProvider`, because these three are internal details of minifi, not real controller services that the user could add to the flow.
>
> I would prefer not to add more refactoring to this PR, as it is too long already. We can do that later.
Makes perfect sense, thanks :)
--
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-unsubscribe@nifi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [nifi-minifi-cpp] fgerlits commented on a diff in pull request #1481: MINIFICPP-1948 Add the UUID to the end of Processor and Controller Service log lines
Posted by GitBox <gi...@apache.org>.
fgerlits commented on code in PR #1481:
URL: https://github.com/apache/nifi-minifi-cpp/pull/1481#discussion_r1053449250
##########
libminifi/include/core/logging/LoggerConfiguration.h:
##########
@@ -133,16 +132,21 @@ class LoggerConfiguration {
class LoggerImpl : public Logger {
public:
- explicit LoggerImpl(std::string name, const std::shared_ptr<LoggerControl> &controller, const std::shared_ptr<spdlog::logger> &delegate)
+ explicit LoggerImpl(std::string name, std::optional<std::string> id, const std::shared_ptr<LoggerControl> &controller, const std::shared_ptr<spdlog::logger> &delegate)
Review Comment:
I wanted to make it generic, so we could add other kinds of stuff, but it's true that it's already specialized to `Identifier` in `getLogger()`. Should I change it to `Identifier` all the way, i.e. in the stored field and the return value of `get_id()`, as well?
--
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-unsubscribe@nifi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [nifi-minifi-cpp] fgerlits commented on pull request #1481: MINIFICPP-1948 Add the UUID to the end of Processor and Controller Service log lines
Posted by GitBox <gi...@apache.org>.
fgerlits commented on PR #1481:
URL: https://github.com/apache/nifi-minifi-cpp/pull/1481#issuecomment-1355126780
> I've found some constructors where the passing of the uuid is still missing
Thanks! I have added most of these, including tensorflow, in 19a655bb6b4fae6feb9bf897d5dc327f09e4225b.
I did not add UUID to the loggers of `ThreadManagementService`, `StandardControllerServiceNode` and `StandardControllerServiceProvider`, because these three are internal details of minifi, not real controller services that the user could add to the flow.
I would prefer not to add more refactoring to this PR, as it is too long already. We can do that later.
--
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-unsubscribe@nifi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [nifi-minifi-cpp] fgerlits commented on a diff in pull request #1481: MINIFICPP-1948 Add the UUID to the end of Processor and Controller Service log lines
Posted by GitBox <gi...@apache.org>.
fgerlits commented on code in PR #1481:
URL: https://github.com/apache/nifi-minifi-cpp/pull/1481#discussion_r1055622646
##########
libminifi/include/core/logging/LoggerConfiguration.h:
##########
@@ -133,16 +132,21 @@ class LoggerConfiguration {
class LoggerImpl : public Logger {
public:
- explicit LoggerImpl(std::string name, const std::shared_ptr<LoggerControl> &controller, const std::shared_ptr<spdlog::logger> &delegate)
+ explicit LoggerImpl(std::string name, std::optional<std::string> id, const std::shared_ptr<LoggerControl> &controller, const std::shared_ptr<spdlog::logger> &delegate)
Review Comment:
I have changed everything to `Identifier` in 9ad1b17b7f52b497d608e1201e0b525b301595d1
--
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-unsubscribe@nifi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [nifi-minifi-cpp] martinzink commented on pull request #1481: MINIFICPP-1948 Add the UUID to the end of Processor and Controller Service log lines
Posted by GitBox <gi...@apache.org>.
martinzink commented on PR #1481:
URL: https://github.com/apache/nifi-minifi-cpp/pull/1481#issuecomment-1354508485
This will be a great addition for our debugging capabilities. :+1:
I've found some constructors where the passing of the uuid is still missing:
[SSLContextService](https://github.com/fgerlits/nifi-minifi-cpp/blob/MINIFICPP-1948_Include-Processor-UUID-in-logs/libminifi/include/controllers/SSLContextService.h#L93)
[ThreadManagementService](https://github.com/fgerlits/nifi-minifi-cpp/blob/MINIFICPP-1948_Include-Processor-UUID-in-logs/libminifi/include/controllers/ThreadManagementService.h#L41-L46)
[ListenSyslog](https://github.com/fgerlits/nifi-minifi-cpp/blob/MINIFICPP-1948_Include-Processor-UUID-in-logs/extensions/standard-processors/processors/ListenSyslog.h#L34)
[ConsumeWindowsEventLog](https://github.com/fgerlits/nifi-minifi-cpp/blob/MINIFICPP-1948_Include-Processor-UUID-in-logs/extensions/windows-event-log/ConsumeWindowsEventLog.cpp#L182)
[CollectorInitiatedSubscription](https://github.com/fgerlits/nifi-minifi-cpp/blob/MINIFICPP-1948_Include-Processor-UUID-in-logs/extensions/windows-event-log/CollectorInitiatedSubscription.cpp#L163)
[DeleteAzureDataLakeStorage](https://github.com/fgerlits/nifi-minifi-cpp/blob/MINIFICPP-1948_Include-Processor-UUID-in-logs/extensions/azure/processors/DeleteAzureDataLakeStorage.h#L52)
[FetchAzureDataLakeStorage](https://github.com/fgerlits/nifi-minifi-cpp/blob/MINIFICPP-1948_Include-Processor-UUID-in-logs/extensions/azure/processors/FetchAzureDataLakeStorage.h#L60)
[StandardControllerServiceNode](https://github.com/fgerlits/nifi-minifi-cpp/blob/MINIFICPP-1948_Include-Processor-UUID-in-logs/libminifi/include/core/controller/StandardControllerServiceNode.h#L37-L43)
[StandardControllerServiceProvider](https://github.com/fgerlits/nifi-minifi-cpp/blob/MINIFICPP-1948_Include-Processor-UUID-in-logs/libminifi/include/core/controller/StandardControllerServiceProvider.h#L52-L62)
(maybe we could refactor some of these so the logger_ isnt initialized in the ctors but rather where it is declared)
also the tensorflow processors [TFApplyGraph](https://github.com/fgerlits/nifi-minifi-cpp/blob/MINIFICPP-1948_Include-Processor-UUID-in-logs/extensions/tensorflow/TFApplyGraph.h#L35), [TFConvertImageToTensor](https://github.com/fgerlits/nifi-minifi-cpp/blob/MINIFICPP-1948_Include-Processor-UUID-in-logs/extensions/tensorflow/TFConvertImageToTensor.h#L35), [TFExtractTopLabels](https://github.com/fgerlits/nifi-minifi-cpp/blob/MINIFICPP-1948_Include-Processor-UUID-in-logs/extensions/tensorflow/TFExtractTopLabels.h#L35) (not sure if we want to do these since these are not really maintained anymore and they might not even compile anymore)
--
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-unsubscribe@nifi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org