Error out if trace already exists unless 'append' option is used

ros2trace/test/ros2trace/test_trace.py

@@ -349,3 +349,34 @@ def test_unknown_context_field(self) -> None:
         self.assertEqual(1, ret)
 
         shutil.rmtree(tmpdir)
+
+    def test_append_trace(self) -> None:
+        tmpdir = self.create_test_tmpdir('test_append_trace')
+
+        # Generate a normal trace
+        ret = self.run_trace_command(
+            ['--path', tmpdir, '--session-name', 'test_append_trace'],
+        )
+        self.assertEqual(0, ret)
+        trace_dir = os.path.join(tmpdir, 'test_append_trace')
+        self.assertTraceExist(trace_dir)
+
+        # Generating another trace with the same path should error out
+        ret = self.run_trace_command(
+            ['--path', tmpdir, '--session-name', 'test_append_trace'],
+        )
+        self.assertEqual(1, ret)
+        self.assertTraceExist(trace_dir)
+
+        # But it should work if we use the '--append-trace' option
+        ret = self.run_trace_command(
+            [
+                '--path', tmpdir,
+                '--session-name', 'test_append_trace',
+                '--append-trace',
+            ],
+        )
+        self.assertEqual(0, ret)
+        self.assertTraceExist(trace_dir)
+
+        shutil.rmtree(tmpdir)

test_tracetools_launch/test/test_tracetools_launch/test_trace_action.py

@@ -20,6 +20,7 @@
 import tempfile
 import textwrap
 from typing import List
+from typing import Optional
 import unittest
 
 from launch import LaunchDescription
@@ -43,41 +44,57 @@ def __init__(self, *args) -> None:
             *args,
         )
 
-    def _assert_launch_no_errors(self, actions) -> None:
+    def setUp(self) -> None:
+        self.assertIsNone(os.environ.get('LD_PRELOAD'))
+
+    def tearDown(self) -> None:
+        if 'LD_PRELOAD' in os.environ:
+            del os.environ['LD_PRELOAD']
+
+    def _assert_launch(self, actions) -> int:
         ld = LaunchDescription(actions)
         ls = LaunchService(debug=True)
         ls.include_launch_description(ld)
-        assert 0 == ls.run()
+        return ls.run()
+
+    def _assert_launch_no_errors(self, actions) -> None:
+        self.assertEqual(0, self._assert_launch(actions), 'expected no errors')
+
+    def _assert_launch_errors(self, actions) -> None:
+        self.assertEqual(1, self._assert_launch(actions), 'expected errors')
 
     def _assert_launch_frontend_no_errors(self, file) -> Trace:
         root_entity, parser = Parser.load(file)
         ld = parser.parse_description(root_entity)
         ls = LaunchService()
         ls.include_launch_description(ld)
-        assert 0 == ls.run()
+        self.assertEqual(0, ls.run(), 'expected no errors')
         trace_action = ld.describe_sub_entities()[0]
         return trace_action
 
     def _check_trace_action(
         self,
         action,
         tmpdir,
-        session_name: str = 'my-session-name',
+        *,
+        session_name: Optional[str] = 'my-session-name',
+        append_trace: bool = False,
         events_ust: List[str] = ['ros2:*', '*'],
         subbuffer_size_ust: int = 524288,
         subbuffer_size_kernel: int = 1048576,
     ) -> None:
-        self.assertEqual(session_name, action.session_name)
+        if session_name is not None:
+            self.assertEqual(session_name, action.session_name)
         self.assertEqual(tmpdir, action.base_path)
         self.assertTrue(action.trace_directory.startswith(tmpdir))
+        self.assertEqual(append_trace, action.append_trace)
         self.assertEqual([], action.events_kernel)
         self.assertEqual(events_ust, action.events_ust)
         self.assertTrue(pathlib.Path(tmpdir).exists())
         self.assertEqual(subbuffer_size_ust, action.subbuffer_size_ust)
         self.assertEqual(subbuffer_size_kernel, action.subbuffer_size_kernel)
 
     def test_action(self) -> None:
-        self.assertIsNone(os.environ.get('LD_PRELOAD'))
         tmpdir = tempfile.mkdtemp(prefix='TestTraceAction__test_action')
 
         # Disable kernel events just to not require kernel tracing for the test
@@ -96,18 +113,18 @@ def test_action(self) -> None:
         self._check_trace_action(action, tmpdir)
 
         shutil.rmtree(tmpdir)
-        del os.environ['LD_PRELOAD']
 
     def test_action_frontend_xml(self) -> None:
-        self.assertIsNone(os.environ.get('LD_PRELOAD'))
         tmpdir = tempfile.mkdtemp(prefix='TestTraceAction__test_frontend_xml')
 
         xml_file = textwrap.dedent(
             r"""
             <launch>
                 <trace
                     session-name="my-session-name"
+                    append-timestamp="false"
                     base-path="{}"
+                    append-trace="true"
                     events-kernel=""
                     events-ust="ros2:* *"
                     subbuffer-size-ust="524288"
@@ -121,21 +138,21 @@ def test_action_frontend_xml(self) -> None:
         with io.StringIO(xml_file) as f:
             trace_action = self._assert_launch_frontend_no_errors(f)
 
-        self._check_trace_action(trace_action, tmpdir)
+        self._check_trace_action(trace_action, tmpdir, append_trace=True)
 
         shutil.rmtree(tmpdir)
-        del os.environ['LD_PRELOAD']
 
     def test_action_frontend_yaml(self) -> None:
-        self.assertIsNone(os.environ.get('LD_PRELOAD'))
         tmpdir = tempfile.mkdtemp(prefix='TestTraceAction__test_frontend_yaml')
 
         yaml_file = textwrap.dedent(
             r"""
             launch:
             - trace:
                 session-name: my-session-name
+                append-timestamp: false
                 base-path: {}
+                append-trace: true
                 events-kernel: ""
                 events-ust: ros2:* *
                 subbuffer-size-ust: 524288
@@ -147,13 +164,11 @@ def test_action_frontend_yaml(self) -> None:
         with io.StringIO(yaml_file) as f:
             trace_action = self._assert_launch_frontend_no_errors(f)
 
-        self._check_trace_action(trace_action, tmpdir)
+        self._check_trace_action(trace_action, tmpdir, append_trace=True)
 
         shutil.rmtree(tmpdir)
-        del os.environ['LD_PRELOAD']
 
     def test_action_context_per_domain(self) -> None:
-        self.assertIsNone(os.environ.get('LD_PRELOAD'))
         tmpdir = tempfile.mkdtemp(prefix='TestTraceAction__test_action_context_per_domain')
 
         action = Trace(
@@ -183,10 +198,8 @@ def test_action_context_per_domain(self) -> None:
         )
 
         shutil.rmtree(tmpdir)
-        del os.environ['LD_PRELOAD']
 
     def test_action_substitutions(self) -> None:
-        self.assertIsNone(os.environ.get('LD_PRELOAD'))
         tmpdir = tempfile.mkdtemp(prefix='TestTraceAction__test_action_substitutions')
 
         self.assertIsNone(os.environ.get('TestTraceAction__event_ust', None))
@@ -231,10 +244,8 @@ def test_action_substitutions(self) -> None:
         shutil.rmtree(tmpdir)
         del os.environ['TestTraceAction__event_ust']
         del os.environ['TestTraceAction__context_field']
-        del os.environ['LD_PRELOAD']
 
     def test_action_ld_preload(self) -> None:
-        self.assertIsNone(os.environ.get('LD_PRELOAD'))
         tmpdir = tempfile.mkdtemp(prefix='TestTraceAction__test_action_ld_preload')
 
         action = Trace(
@@ -285,7 +296,70 @@ def test_action_ld_preload(self) -> None:
         self.assertTrue(any(p.endswith('liblttng-ust-dl.so') for p in paths))
 
         shutil.rmtree(tmpdir)
-        del os.environ['LD_PRELOAD']
+
+    def test_append_timestamp(self) -> None:
+        tmpdir = tempfile.mkdtemp(prefix='TestTraceAction__test_append_timestamp')
+
+        action = Trace(
+            session_name='my-session-name',
+            append_timestamp=True,
+            base_path=tmpdir,
+            events_kernel=[],
+            events_ust=[
+                'ros2:*',
+                '*',
+            ],
+            subbuffer_size_ust=524288,
+            subbuffer_size_kernel=1048576,
+        )
+        self._assert_launch_no_errors([action])
+        self._check_trace_action(action, tmpdir, session_name=None)
+        # Session name should start with the given prefix and end with the timestamp, but don't
+        # bother validating the timestamp here
+        self.assertTrue(action.session_name.startswith('my-session-name-'))
+        self.assertNotEqual('my-session-name-', action.session_name)
+
+        shutil.rmtree(tmpdir)
+
+    def test_append_trace(self) -> None:
+        tmpdir = tempfile.mkdtemp(prefix='TestTraceAction__test_append_trace')
+
+        # Generate a normal trace
+        action = Trace(
+            session_name='my-session-name',
+            base_path=tmpdir,
+            append_trace=False,
+            events_kernel=[],
+            events_ust=[
+                'ros2:*',
+                '*',
+            ],
+            subbuffer_size_ust=524288,
+            subbuffer_size_kernel=1048576,
+        )
+        self._assert_launch_no_errors([action])
+        self._check_trace_action(action, tmpdir, append_trace=False)
+
+        # Generating another trace with the same path should error out
+        self._assert_launch_errors([action])
+
+        # But it should work if we use the append_trace option
+        action = Trace(
+            session_name='my-session-name',
+            base_path=tmpdir,
+            append_trace=True,
+            events_kernel=[],
+            events_ust=[
+                'ros2:*',
+                '*',
+            ],
+            subbuffer_size_ust=524288,
+            subbuffer_size_kernel=1048576,
+        )
+        self._assert_launch_no_errors([action])
+        self._check_trace_action(action, tmpdir, append_trace=True)
+
+        shutil.rmtree(tmpdir)
 
 
 if __name__ == '__main__':

tracetools_launch/tracetools_launch/action.py

@@ -109,6 +109,7 @@ def __init__(
         session_name: SomeSubstitutionsType,
         append_timestamp: bool = False,
         base_path: Optional[SomeSubstitutionsType] = None,
+        append_trace: bool = False,
         events_ust: Iterable[SomeSubstitutionsType] = names.DEFAULT_EVENTS_ROS,
         events_kernel: Iterable[SomeSubstitutionsType] = [],
         context_fields:
@@ -135,6 +136,8 @@ def __init__(
         :param append_timestamp: whether to append timestamp to the session name
         :param base_path: the path to the base directory in which to create the session directory,
             or `None` for default
+        :param append_trace: whether to append to the trace directory if it already exists,
+            otherwise an error is reported
         :param events_ust: the list of ROS UST events to enable
         :param events_kernel: the list of kernel events to enable
         :param context_fields: the names of context fields to enable
@@ -152,6 +155,7 @@ def __init__(
         self.__session_name = normalize_to_list_of_substitutions(session_name)
         self.__base_path = base_path \
             if base_path is None else normalize_to_list_of_substitutions(base_path)
+        self.__append_trace = append_trace
         self.__trace_directory = None
         self.__events_ust = [normalize_to_list_of_substitutions(x) for x in events_ust]
         self.__events_kernel = [normalize_to_list_of_substitutions(x) for x in events_kernel]
@@ -174,6 +178,10 @@ def session_name(self):
     def base_path(self):
         return self.__base_path
 
+    @property
+    def append_trace(self):
+        return self.__append_trace
+
     @property
     def trace_directory(self):
         return self.__trace_directory
@@ -272,6 +280,10 @@ def parse(cls, entity: Entity, parser: Parser):
         base_path = entity.get_attr('base-path', optional=True)
         if base_path:
             kwargs['base_path'] = parser.parse_substitution(base_path)
+        append_trace = entity.get_attr(
+            'append-trace', data_type=bool, optional=True, can_be_str=False)
+        if append_trace is not None:
+            kwargs['append_trace'] = append_trace
         # Make sure to handle empty strings and replace with empty lists,
         # otherwise an empty string enables all events
         events_ust = entity.get_attr('events-ust', optional=True)
@@ -402,6 +414,7 @@ def _setup(self) -> bool:
             self.__trace_directory = lttng.lttng_init(
                 session_name=self.__session_name,
                 base_path=self.__base_path,
+                append_trace=self.__append_trace,
                 ros_events=self.__events_ust,
                 kernel_events=self.__events_kernel,
                 context_fields=self.__context_fields,
@@ -436,6 +449,7 @@ def __repr__(self):
             'Trace('
             f'session_name={self.__session_name}, '
             f'base_path={self.__base_path}, '
+            f'append_trace={self.__append_trace}, '
             f'trace_directory={self.__trace_directory}, '
             f'events_ust={self.__events_ust}, '
             f'events_kernel={self.__events_kernel}, '

tracetools_trace/tracetools_trace/tools/args.py

@@ -76,3 +76,6 @@ def add_arguments(parser: argparse.ArgumentParser) -> None:
     parser.add_argument(
         '-l', '--list', dest='list', action='store_true',
         help='display lists of enabled events and context names (default: %(default)s)')
+    parser.add_argument(
+        '-a', '--append-trace', dest='append_trace', action='store_true',
+        help='append to trace if it already exists, otherwise error out (default: %(default)s)')

tracetools_trace/tracetools_trace/tools/lttng_impl.py

@@ -68,6 +68,7 @@ def setup(
     *,
     session_name: str,
     base_path: str,
+    append_trace: bool = False,
     ros_events: Union[List[str], Set[str]] = DEFAULT_EVENTS_ROS,
     kernel_events: Union[List[str], Set[str]] = [],
     context_fields: Union[List[str], Set[str], Dict[str, List[str]]] = DEFAULT_CONTEXT,
@@ -89,6 +90,8 @@ def setup(
     :param session_name: the name of the session
     :param base_path: the path to the directory in which to create the tracing session directory,
         which will be created if needed
+    :param append_trace: whether to append to the trace directory if it already exists, otherwise
+        an error is reported
     :param ros_events: list of ROS events to enable
     :param kernel_events: list of kernel events to enable
     :param context_fields: the names of context fields to enable
@@ -106,6 +109,11 @@ def setup(
     # Validate parameters
     if not session_name:
         raise RuntimeError('empty session name')
+    # Resolve full tracing directory path
+    full_path = os.path.join(base_path, session_name)
+    if os.path.isdir(full_path) and not append_trace:
+        raise RuntimeError(
+            f'trace directory already exists, use the append option to append to it: {full_path}')
 
     # Check if there is a session daemon running
     if lttng.session_daemon_alive() == 0:
@@ -179,9 +187,8 @@ def setup(
         channel_kernel.attr.output = lttng.EVENT_MMAP
         events_list_kernel = _create_events(kernel_events)
 
-    # Resolve full tracing directory path and create session
+    # Create session
     # LTTng will create the parent directories if needed
-    full_path = os.path.join(base_path, session_name)
     _create_session(session_name, full_path)
 
     # Handles, channels, events