dstandish commented on code in PR #38514:
URL: https://github.com/apache/airflow/pull/38514#discussion_r1550099456


##########
airflow/jobs/scheduler_job_runner.py:
##########
@@ -270,8 +270,10 @@ def _debug_dump(self, signum: int, frame: FrameType | 
None) -> None:
 
         self.log.info("%s\n%s received, printing debug\n%s", "-" * 80, 
sig_name, "-" * 80)
 
-        self.job.executor.debug_dump()
-        self.log.info("-" * 80)
+        for executor in self.job.executors:
+            self.log.info("Debug dump for the executor %s", executor)
+            executor.debug_dump()
+            self.log.info("-" * 80)

Review Comment:
   Yeah to explain the motivation, I think is that commonly nowadays we are 
using logging tools where we search and filter messages to find what we are 
looking for, where in an earlier time we were perhaps more likely to just copy 
the log file locally and open it up and search through.  And so I think it 
becomes more important to ensure that each individual log message has the right 
context info attached to it and i think probably also you want to have more 
content on fewer lines -- whereas if you are just opening the file you want 
stuff pretty printed and not long lines. 
   
   Like, the base executor debug dump as an example, if you are searching 
through splunk or something you probably would prefer all of this in one long 
message instead of three, with appropriate tagging to identify it all as the 
log message for the single event of the debug dump :
   ```
       def debug_dump(self):
           """Get called in response to SIGUSR2 by the scheduler."""
           self.log.info(
               "executor.queued (%d)\n\t%s",
               len(self.queued_tasks),
               "\n\t".join(map(repr, self.queued_tasks.items())),
           )
           self.log.info("executor.running (%d)\n\t%s", len(self.running), 
"\n\t".join(map(repr, self.running)))
           self.log.info(
               "executor.event_buffer (%d)\n\t%s",
               len(self.event_buffer),
               "\n\t".join(map(repr, self.event_buffer.items())),
           )
   ```
   
   Then you would not need `self.log.info("Debug dump for the executor %s", 
executor)` either, and the `---` does not add anything in that context.  And I 
think we should probably move in that direction over time.



-- 
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: commits-unsubscr...@airflow.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org

Reply via email to