feat: Add APPMAP_DISPLAY_LABELED_PARAMS to capture params for labeled functions

_appmap/env.py

@@ -47,6 +47,8 @@ def __init__(self, env=None, cwd=None):
         self._enabled = enabled is not None and enabled.lower() != "false"
         display_params = self._env.get("_APPMAP_DISPLAY_PARAMS", None)
         self._display_params = display_params is not None and display_params.lower() != "false"
+        display_labeled_params = self._env.get("_APPMAP_DISPLAY_LABELED_PARAMS", None)
+        self._display_labeled_params = display_labeled_params is not None and display_labeled_params.lower() != "false"
 
         logger = logging.getLogger(__name__)
         # The user shouldn't set APPMAP_OUTPUT_DIR, but some tests depend on being able to use it.
@@ -134,6 +136,10 @@ def is_appmap_repo(self):
     def display_params(self):
         return self._display_params
 
+    @property
+    def display_labeled_params(self):
+        return self._display_labeled_params
+
     def getLogger(self, name) -> trace_logger.TraceLogger:
         return cast(trace_logger.TraceLogger, logging.getLogger(name))
 

_appmap/event.py

@@ -45,13 +45,21 @@ def reset(cls):
         cls._next_thread_id = 0
 
 
-def display_string(val):
+def _should_display(has_labels):
+    if Env.current.display_params:
+        return True
+    if has_labels and Env.current.display_labeled_params:
+        return True
+    return False
+
+
+def display_string(val, has_labels=False):
     # If we're asked to display parameters, make a best-effort attempt
     # to get a string value for the parameter using repr(). If parameter
     # display is disabled, or repr() has raised, just formulate a value
     # from the class and id.
     value = None
-    if Env.current.display_params:
+    if _should_display(has_labels):
         try:
             value = repr(val)
         except Exception:  # pylint: disable=broad-except
@@ -108,12 +116,12 @@ def _describe_schema(name, val, depth, max_depth):
     return ret
 
 
-def describe_value(name, val, max_depth=5):
+def describe_value(name, val, max_depth=5, has_labels=False):
     ret = {
         "object_id": id(val),
-        "value": display_string(val),
+        "value": display_string(val, has_labels=has_labels),
     }
-    if Env.current.display_params:
+    if _should_display(has_labels):
         ret.update(_describe_schema(name, val, 0, max_depth))
 
     if any(_is_list_or_dict(type(val))):
@@ -170,9 +178,9 @@ def __init__(self, sigp):
     def __repr__(self):
         return "<Param name: %s kind: %s>" % (self.name, self.kind)
 
-    def to_dict(self, value):
+    def to_dict(self, value, has_labels=False):
         ret = {"kind": self.kind}
-        ret.update(describe_value(self.name, value))
+        ret.update(describe_value(self.name, value, has_labels=has_labels))
         return ret
 
 def _get_name_parts(filterable):
@@ -247,7 +255,7 @@ def make_params(filterable):
         return [Param(p) for p in sig.parameters.values()]
 
     @staticmethod
-    def set_params(params, instance, args, kwargs):
+    def set_params(params, instance, args, kwargs, has_labels=False):
         # pylint: disable=too-many-branches
         # Note that set_params expects args and kwargs as a tuple and
         # dict, respectively. It operates on them as collections, so
@@ -295,7 +303,7 @@ def set_params(params, instance, args, kwargs):
                 # If all the parameter types are handled, this
                 # shouldn't ever happen...
                 raise RuntimeError("Unknown parameter with desc %s" % (repr(p)))
-            ret.append(p.to_dict(value))
+            ret.append(p.to_dict(value, has_labels=has_labels))
         return ret
 
     @property
@@ -497,13 +505,13 @@ def __init__(self, parent_id, elapsed):
 class FuncReturnEvent(ReturnEvent):
     __slots__ = ["return_value"]
 
-    def __init__(self, parent_id, elapsed, return_value):
+    def __init__(self, parent_id, elapsed, return_value, has_labels=False):
         super().__init__(parent_id, elapsed)
         # Import here to prevent circular dependency
         # pylint: disable=import-outside-toplevel
         from _appmap.instrument import recording_disabled # noqa: F401
         with recording_disabled():
-            self.return_value = describe_value(None, return_value)
+            self.return_value = describe_value(None, return_value, has_labels=has_labels)
 
 
 class HttpResponseEvent(ReturnEvent):

_appmap/instrument.py

@@ -84,7 +84,8 @@ def call_instrumented(f, instance, args, kwargs):
 
     with recording_disabled():
         logger.trace("%s args %s kwargs %s", f.fn, args, kwargs)
-        params = CallEvent.set_params(f.params, instance, args, kwargs)
+        has_labels = bool(f.make_call_event.keywords.get("labels"))
+        params = CallEvent.set_params(f.params, instance, args, kwargs, has_labels=has_labels)
         call_event = f.make_call_event(parameters=params)
     Recorder.add_event(call_event)
     call_event_id = call_event.id
@@ -95,7 +96,8 @@ def call_instrumented(f, instance, args, kwargs):
         elapsed_time = time.time() - start_time
 
         return_event = event.FuncReturnEvent(
-            return_value=ret, parent_id=call_event_id, elapsed=elapsed_time
+            return_value=ret, parent_id=call_event_id, elapsed=elapsed_time,
+            has_labels=has_labels
         )
         Recorder.add_event(return_event)
         return ret

_appmap/test/conftest.py

@@ -69,6 +69,7 @@ def pytest_runtest_setup(item):
             env.pop("_APPMAP", None)
 
     env["_APPMAP_DISPLAY_PARAMS"] = env.get("APPMAP_DISPLAY_PARAMS", "true")
+    env["_APPMAP_DISPLAY_LABELED_PARAMS"] = env.get("APPMAP_DISPLAY_LABELED_PARAMS", "true")
 
     _appmap.initialize(env=env)  # pylint: disable=protected-access
 

_appmap/test/data/example_class.py

@@ -56,6 +56,10 @@ def test_exception(self):
     def labeled_method(self):
         return "super important"
 
+    @appmap.labels("super", "important")
+    def labeled_method_with_param(self, p):
+        return p
+
     @staticmethod
     @wrap_fn
     def wrapped_static_method():

_appmap/test/test_events.py

@@ -117,6 +117,57 @@ def test_describe_return_value_recursion_protection(self):
             "return_self"
         ]
 
+    @pytest.mark.appmap_enabled(env={"APPMAP_DISPLAY_PARAMS": "false"})
+    def test_labeled_params_displayed_by_default(self):
+        """When display_params is off but display_labeled_params is on (default),
+        labeled functions should still have their params displayed via repr()."""
+        r = appmap.Recording()
+        with r:
+            from example_class import ExampleClass  # pylint: disable=import-outside-toplevel
+
+            result = ExampleClass().labeled_method_with_param("hello")
+
+        assert result == "hello"
+        call_event = r.events[0]
+        # Parameter value should be the repr, not the opaque object string
+        assert call_event.parameters[0]["value"] == "'hello'"
+        # Return value should also be displayed
+        return_event = r.events[1]
+        assert return_event.return_value["value"] == "'hello'"
+
+    @pytest.mark.appmap_enabled(
+        env={
+            "APPMAP_DISPLAY_PARAMS": "false",
+            "APPMAP_DISPLAY_LABELED_PARAMS": "false",
+        }
+    )
+    def test_labeled_params_not_displayed_when_disabled(self):
+        """When both display_params and display_labeled_params are off,
+        labeled functions should NOT have their params displayed."""
+        r = appmap.Recording()
+        with r:
+            from example_class import ExampleClass  # pylint: disable=import-outside-toplevel
+
+            ExampleClass().labeled_method_with_param("hello")
+
+        call_event = r.events[0]
+        # Parameter value should be the opaque object string
+        assert "object at" in call_event.parameters[0]["value"]
+
+    @pytest.mark.appmap_enabled(env={"APPMAP_DISPLAY_PARAMS": "false"})
+    def test_unlabeled_params_not_displayed(self):
+        """When display_params is off, unlabeled functions should NOT
+        have their params displayed (even though display_labeled_params is on)."""
+        r = appmap.Recording()
+        with r:
+            from example_class import ExampleClass  # pylint: disable=import-outside-toplevel
+
+            ExampleClass().instance_with_param("hello")
+
+        call_event = r.events[0]
+        # Parameter value should be the opaque object string
+        assert "object at" in call_event.parameters[0]["value"]
+
     # There should be an exception return event generated even when the raised exception is a
     # BaseException.
     def test_exception_event_with_base_exception(self):

appmap/__init__.py

@@ -15,6 +15,8 @@
         os.environ.setdefault("_APPMAP", _enabled)
         _display_params = os.environ.get("APPMAP_DISPLAY_PARAMS", "false")
         os.environ.setdefault("_APPMAP_DISPLAY_PARAMS", _display_params)
+        _display_labeled_params = os.environ.get("APPMAP_DISPLAY_LABELED_PARAMS", "true")
+        os.environ.setdefault("_APPMAP_DISPLAY_LABELED_PARAMS", _display_labeled_params)
 
         from _appmap import generation  # noqa: F401
         from _appmap.env import Env  # noqa: F401
@@ -56,9 +58,11 @@ def enabled():
     else:
         os.environ.pop("_APPMAP", None)
         os.environ.pop("_APPMAP_DISPLAY_PARAMS", None)
+        os.environ.pop("_APPMAP_DISPLAY_LABELED_PARAMS", None)
 else:
     os.environ.setdefault("_APPMAP", "false")
     os.environ.setdefault("_APPMAP_DISPLAY_PARAMS", "false")
+    os.environ.setdefault("_APPMAP_DISPLAY_LABELED_PARAMS", "false")
 
 if not _recording_exported:
     # Client code that imports appmap.Recording should run correctly