[GitHub] [nifi] turcsanyip commented on a change in pull request #4925: NIFI-8356: Add unit test for LongRunningTaskMonitor.

[GitBox <gi...@apache.org>]

turcsanyip commented on a change in pull request #4925:
URL: https://github.com/apache/nifi/pull/4925#discussion_r600584036



##########
File path: nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-framework-core/src/main/java/org/apache/nifi/components/monitor/LongRunningTaskMonitor.java
##########
@@ -72,6 +73,16 @@ public void run() {
             }
         }
 
-        LOGGER.info("Active threads: {}; Long running threads: {}", activeThreadCount, longRunningThreadCount);
+        getLogger().info("Active threads: {}; Long running threads: {}", activeThreadCount, longRunningThreadCount);
+    }
+
+    @VisibleForTesting

Review comment:
       The main purpose of this class to log info about long running (possible stuck) tasks in the nifi log file to make troubleshooting easier. The nifi log is the only persistent storage of these messages because the bulletins disappear after a while. That's why I would assert these statements.
   
   Additionally, the messages are shown on the UI in two places: controller and processor level bulletins. `EventReport.reportEvent()` handles the controller level bulletin but the processor level bulletin is triggered by log items written by the processors logger. That's why we need assert that log call.
   
   Asserting the last "Active threads:" summary log may be unnecessary but after checking the other logs I added it too.




-- 
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:
users@infra.apache.org