feat: implement syskit_on_log_rotation hook to trigger rotation on non-orogen loggers

lib/syskit/roby_app/plugin.rb

@@ -205,6 +205,8 @@ def syskit_log_transfer_prepare
                           "cannot enable log transfer without log rotation"
                 end
 
+                Roby.warn "built-in log transfer is deprecated. Use the transfer " \
+                          "functionality of `syskit log_runtime_archive` instead"
                 conf = Syskit.conf.log_transfer
                 conf.target_dir ||= log_dir
                 @syskit_log_transfer_manager = LogTransferManager.new(conf)
@@ -309,7 +311,7 @@ def self.prepare(app)
 
                 if Syskit.conf.log_rotation_period
                     @log_rotation_poll_handler =
-                        app.execution_engine.every(Syskit.conf.log_rotation_period) do
+                        app.execution_engine.every(Syskit.conf.log_rotation_period, immediate: false) do
                             app.syskit_log_rotation_poll_handler
                         end
                 end
@@ -331,6 +333,33 @@ def syskit_log_rotation_poll_handler
                 syskit_log_transfer_poll_state
             end
 
+            # @api private
+            #
+            # Call the blocks registered with {#syskit_on_log_rotation} while doing
+            # error handling
+            #
+            # @see syskit_rotate_logs
+            def syskit_call_rotation_handlers
+                @syskit_log_rotation_handlers&.delete_if do |h|
+                    h.call
+                    false
+                rescue StandardError => e
+                    ::Robot.warn "disabling log rotation handler #{h} because it raised"
+                    Roby.log_exception_with_backtrace(e, ::Robot, :warn)
+                    true
+                end
+            end
+
+            # Register a block called during log rotation
+            #
+            # @return [#dispose] a disposable that will de-register the callback
+            # @see syskit_rotate_logs
+            def syskit_on_log_rotation(&block)
+                @syskit_log_rotation_handlers ||= []
+                @syskit_log_rotation_handlers << block
+                Roby.disposable { @syskit_log_rotation_handlers.delete(block) }
+            end
+
             # Hook called by the main application to undo what {.prepare} did
             def self.shutdown(app)
                 remaining = Orocos.each_process.to_a
@@ -1066,14 +1095,41 @@ def self.setup_rest_interface(app, rest_api)
                 rest_api.mount REST_API => "/syskit"
             end
 
+            # Perform log rotation
+            #
+            # Syskit provides two mechanisms to rotate logs. An in-plan mechanism,
+            # and a callback-based mechanism.
+            #
+            # In-plan: the method looks for any task that provides the
+            # `Syskit::LoggerService` task service. See below for the requirements on
+            # these tasks.
+            #
+            # The callbacks registered via {#syskit_on_log_rotation} are then called
+            # in sequence. Callbacks that raise are autoamtically disabled.
+            #
+            # The tasks processed by the in-plan step are expected to have two methods:
+            # `log_server_name` and `rotate_log`. `log_server_name` returns a key that
+            # is used for log transfer. In-process log transfer is now deprecated in
+            # favor of `syskit log_runtime_archive`, so this method may return any value
+            # as long as the in-process transfer is disabled. `rotate_log` must return
+            # the list of the names of the logs that have been closed because of the
+            # rotation (i.e. the 'old files')
+            #
+            # @return [Hash<Object, Array>] a map from the keys returned by the
+            #   log_server_name methods to the list of old files rotated under
+            #   that key
             def syskit_rotate_logs
-                plan.find_tasks(Syskit::LoggerService)
+                result =
+                    plan.find_tasks(Syskit::LoggerService)
                     .running.each_with_object({}) do |task, rotated_logs|
                         process_server = Syskit.conf.process_server_config_for(
                             task.log_server_name
                         )
                         (rotated_logs[process_server] ||= []).concat(task.rotate_log)
                     end
+
+                syskit_call_rotation_handlers
+                result
             end
         end
     end

test/roby_app/test_plugin.rb

@@ -250,6 +250,33 @@ def rotate_log
                     assert_equal({ stubs => ["old_log_file.log"] }, rotated_logs)
                 end
 
+                it "calls the blocks registered with on_log_rotation" do
+                    mock = flexmock
+                    mock.should_receive(:called).once
+                    app.syskit_on_log_rotation { mock.called }
+                    app.syskit_rotate_logs
+                end
+
+                it "stop calling the blocks registered when disposed" do
+                    mock = flexmock
+                    mock.should_receive(:called).never
+                    handler = app.syskit_on_log_rotation { mock.called }
+                    handler.dispose
+                    app.syskit_rotate_logs
+                end
+
+                it "stop calling the blocks registered when the block raised" do
+                    mock = flexmock
+                    mock.should_receive(:called).once
+                    flexmock(::Robot).should_receive(:warn).at_least.once
+                    app.syskit_on_log_rotation do
+                        mock.called
+                        raise "some error"
+                    end
+                    app.syskit_rotate_logs
+                    app.syskit_rotate_logs
+                end
+
                 it "returns an empty list of process servers " \
                    "if log transfer is disabled" do
                     conf = Syskit.conf.process_server_config_for("localhost")