feat: Add APPMAP_DISPLAY_LABELED_PARAMS to capture params for labeled functions

_appmap/env.py

@@ -45,8 +45,16 @@ def __init__(self, env=None, cwd=None):
         # them.
         enabled = self._env.get("_APPMAP", 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_params = self._env.get("_APPMAP_DISPLAY_PARAMS", "labeled").lower()
+        if display_params == "true":
+            self._display_params = True
+            self._display_labeled_params = True
+        elif display_params == "false":
+            self._display_params = False
+            self._display_labeled_params = False
+        else: # "labeled" or "auto" or anything else defaults to labeled
+            self._display_params = False
+            self._display_labeled_params = True
 
         logger = logging.getLogger(__name__)
         # The user shouldn't set APPMAP_OUTPUT_DIR, but some tests depend on being able to use it.
@@ -134,6 +142,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,13 @@ def reset(cls):
         cls._next_thread_id = 0
 
 
-def display_string(val):
+def display_string(val, display_value=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 display_value:
         try:
             value = repr(val)
         except Exception:  # pylint: disable=broad-except
@@ -79,7 +79,7 @@ def _is_list_or_dict(val_type):
     return issubclass(val_type, list), issubclass(val_type, dict)
 
 
-def _describe_schema(name, val, depth, max_depth):
+def _describe_schema(name, val, depth, max_depth, display_value=False):
 
     val_type = type(val)
 
@@ -88,6 +88,9 @@ def _describe_schema(name, val, depth, max_depth):
         ret["name"] = name
     ret["class"] = fqname(val_type)
 
+    if not display_value:
+        return ret
+
     islist, isdict = _is_list_or_dict(val_type)
     if not (islist or isdict) or (depth >= max_depth and isdict):
         return ret
@@ -100,23 +103,25 @@ def _describe_schema(name, val, depth, max_depth):
         elts = val.items()
         schema_key = "properties"
 
-    schema = [_describe_schema(k, v, depth + 1, max_depth) for k, v in elts]
+    schema = [
+        _describe_schema(k, v, depth + 1, max_depth, display_value=display_value)
+        for k, v in elts
+    ]
     # schema will be [None] if depth is exceeded, don't use it
     if any(schema):
         ret[schema_key] = schema
 
     return ret
 
 
-def describe_value(name, val, max_depth=5):
-    ret = {
+def describe_value(name, val, max_depth=5, display_value=False):
+    ret = _describe_schema(name, val, 0, max_depth, display_value=display_value)
+    ret.update({
         "object_id": id(val),
-        "value": display_string(val),
-    }
-    if Env.current.display_params:
-        ret.update(_describe_schema(name, val, 0, max_depth))
+        "value": display_string(val, display_value=display_value),
+    })
 
-    if any(_is_list_or_dict(type(val))):
+    if display_value and any(_is_list_or_dict(type(val))):
         ret["size"] = len(val)
 
     return ret
@@ -170,9 +175,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, display_value=False):
         ret = {"kind": self.kind}
-        ret.update(describe_value(self.name, value))
+        ret.update(describe_value(self.name, value, display_value=display_value))
         return ret
 
 def _get_name_parts(filterable):
@@ -247,7 +252,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, display_value=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 +300,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, display_value=display_value))
         return ret
 
     @property
@@ -405,8 +410,9 @@ def message_parameters(self):
 
     @message_parameters.setter
     def message_parameters(self, params):
+        display_params = Env.current.display_params
         for name, value in params.items():
-            message_object = describe_value(name, value)
+            message_object = describe_value(name, value, display_value=display_params)
             self.message.append(message_object)
 
 
@@ -497,13 +503,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, display_value=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, display_value=display_value)
 
 
 class HttpResponseEvent(ReturnEvent):

_appmap/instrument.py

@@ -70,7 +70,7 @@ def saved_shallow_rule():
 
 
 _InstrumentedFn = namedtuple(
-    "_InstrumentedFn", "fn fntype instrumented_fn make_call_event params"
+    "_InstrumentedFn", "fn fntype instrumented_fn make_call_event params display_params"
 )
 
 
@@ -84,7 +84,9 @@ 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)
+        params = CallEvent.set_params(
+            f.params, instance, args, kwargs, display_value=f.display_params
+        )
         call_event = f.make_call_event(parameters=params)
     Recorder.add_event(call_event)
     call_event_id = call_event.id
@@ -95,7 +97,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,
+            display_value=f.display_params
         )
         Recorder.add_event(return_event)
         return ret
@@ -120,9 +123,16 @@ def instrument(filterable):
     """return an instrumented function"""
     logger.debug("hooking %s", filterable.fqname)
 
+    # note this has to happen before CallEvent.make, which clears this attribute
+    has_labels = hasattr(filterable.obj, "_appmap_labels")
+
     make_call_event = event.CallEvent.make(filterable)
     params = CallEvent.make_params(filterable)
 
+    display_params = Env.current.display_params or (
+        has_labels and Env.current.display_labeled_params
+    )
+
     # django depends on being able to find the cache_clear attribute
     # on functions. (You can see this by trying to map
     # https://github.com/chicagopython/chypi.org.) Make sure it gets
@@ -132,7 +142,8 @@ def instrument(filterable):
     def instrumented_fn(wrapped, instance, args, kwargs):
         with saved_shallow_rule():
             f = _InstrumentedFn(
-                wrapped, filterable.fntype, instrumented_fn, make_call_event, params
+                wrapped, filterable.fntype, instrumented_fn, make_call_event, params,
+                display_params
             )
             return call_instrumented(f, instance, args, kwargs)
 

_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_describe_value.py

@@ -24,7 +24,7 @@ def value(self):
         return {"id": 1, "contents": "some text"}
 
     def test_one_level_schema(self, value):
-        actual = describe_value(None, value)
+        actual = describe_value(None, value, display_value=True)
         assert actual == DictIncluding(
             {
                 "properties": [
@@ -34,14 +34,21 @@ def test_one_level_schema(self, value):
             }
         )
 
+    def test_one_level_schema_display_false(self, value):
+        actual = describe_value(None, value, display_value=False)
+        assert "properties" not in actual
+        assert actual["class"] == "builtins.dict"
+        assert "builtins.dict object at" in actual["value"]
+        assert actual["object_id"] == id(value)
+
 
 class TestNestedDictValue:
     @pytest.fixture
     def value(self):
         return {"page": {"page_number": 1, "page_size": 20, "total": 2383}}
 
     def test_two_level_schema(self, value):
-        actual = describe_value(None, value)
+        actual = describe_value(None, value, display_value=True)
         assert actual == DictIncluding(
             {
                 "properties": [
@@ -60,7 +67,7 @@ def test_two_level_schema(self, value):
 
     def test_respects_max_depth(self, value):
         expected = {"properties": [{"name": "page", "class": "builtins.dict"}]}
-        actual = describe_value(None, value, max_depth=1)
+        actual = describe_value(None, value, max_depth=1, display_value=True)
         assert actual == DictIncluding(expected)
 
 
@@ -70,7 +77,7 @@ def value(self):
         return [{"id": 1, "contents": "some text"}, {"id": 2}]
 
     def test_an_array_containing_schema(self, value):
-        actual = describe_value(None, value)
+        actual = describe_value(None, value, display_value=True)
         assert actual["class"] == "builtins.list"
         assert actual["items"][0] == DictIncluding(
             {
@@ -88,14 +95,21 @@ def test_an_array_containing_schema(self, value):
             }
         )
 
+    def test_an_array_display_false(self, value):
+        actual = describe_value(None, value, display_value=False)
+        assert "items" not in actual
+        assert actual["class"] == "builtins.list"
+        assert "builtins.list object at" in actual["value"]
+        assert actual["object_id"] == id(value)
+
 
 class TestNestedArrays:
     @pytest.fixture
     def value(self):
         return [[["one"]]]
 
     def test_arrays_ignore_max_depth(self, value):
-        actual = describe_value(None, value, max_depth=1)
+        actual = describe_value(None, value, max_depth=1, display_value=True)
         expected = {
             "class": "builtins.list",
             "items": [

_appmap/test/test_events.py

@@ -52,6 +52,7 @@ def test_recursion_protection(self):
         # is working
         assert True
 
+    @pytest.mark.appmap_enabled(env={"APPMAP_DISPLAY_PARAMS": "true"})
     def test_when_str_raises(self, mocker):
         r = appmap.Recording()
         with r:
@@ -68,6 +69,7 @@ def test_when_str_raises(self, mocker):
         actual_value = r.events[0].parameters[0]["value"]
         assert expected_value == actual_value
 
+    @pytest.mark.appmap_enabled(env={"APPMAP_DISPLAY_PARAMS": "true"})
     def test_when_both_raise(self, mocker):
         r = appmap.Recording()
         with r:
@@ -117,6 +119,60 @@ def test_describe_return_value_recursion_protection(self):
             "return_self"
         ]
 
+    @pytest.mark.appmap_enabled(env={"APPMAP_DISPLAY_PARAMS": None})
+    def test_labeled_params_displayed_by_default(self):
+        """When display_params is 'labeled' (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")
+            ExampleClass().instance_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'"
+
+        # Unlabeled method should not have its params displayed, even in the same recording
+        call_event_unlabeled = r.events[2]
+        assert "object at" in call_event_unlabeled.parameters[0]["value"]
+
+    @pytest.mark.appmap_enabled(
+        env={
+            "APPMAP_DISPLAY_PARAMS": "false",
+        }
+    )
+    def test_labeled_params_not_displayed_when_disabled(self):
+        """When display_params is 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": "labeled"})
+    def test_unlabeled_params_not_displayed(self):
+        """When display_params is 'labeled', unlabeled functions should NOT
+        have their params displayed."""
+        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/test/test_params.py

@@ -33,7 +33,9 @@ def prepare(cls, ffn):
 
         def wrapped_fn(_, instance, args, kwargs):
             return make_call_event(
-                parameters=CallEvent.set_params(params, instance, args, kwargs)
+                parameters=CallEvent.set_params(
+                    params, instance, args, kwargs, display_value=True
+                )
             )
 
         return wrapped_fn

_appmap/web_framework.py

@@ -44,7 +44,7 @@ class TemplateEvent(Event):  # pylint: disable=too-few-public-methods
 
     def __init__(self, path, instance=None):
         super().__init__("call")
-        self.receiver = describe_value(None, instance)
+        self.receiver = describe_value(None, instance, display_value=Env.current.display_params)
         self.path = root_relative_path(path)
 
     def to_dict(self, attrs=None):