Merge pull request #180 from getappmap/record-by-default_20221017

Fix record-by-defauilt

Author: apotterri

appmap/_implementation/__init__.py

@@ -8,12 +8,12 @@
 def initialize(**kwargs):
     check_py_version()
     appmapenv.initialize(**kwargs)
+    DetectEnabled.initialize()
     event.initialize()
     importer.initialize()
     recorder.initialize()
     configuration.initialize()  # needs to be initialized after recorder
     metadata.initialize()
-    DetectEnabled.initialize()
 
 
 initialize()

appmap/_implementation/configuration.py

@@ -15,6 +15,8 @@
 import yaml
 from yaml.parser import ParserError
 
+from appmap._implementation.detect_enabled import DetectEnabled
+
 from ..labeling import presets as label_presets
 from . import utils
 from .env import Env
@@ -226,6 +228,9 @@ def _load_config(self):
             Env.current.enabled = False
 
     def write_config_file(self, filepath, config):
+        # HACK: don't scribble on the repo when testing
+        if DetectEnabled.is_appmap_repo():
+            return
         basedir = filepath.parent
         if not basedir.exists():
             basedir.mkdir(parents=True, exist_ok=True)

appmap/_implementation/detect_enabled.py

@@ -58,14 +58,22 @@ def should_enable(cls, recording_method):
         if recording_method in cls._detected_for_method:
             return cls._detected_for_method[recording_method]
         else:
-            message, enabled = cls.detect_should_enable(recording_method)
+            message, enabled = cls._detect_should_enable(recording_method)
             cls._detected_for_method[recording_method] = enabled
             if enabled:
                 logger.warning(dedent(f"AppMap recording is enabled because {message}"))
             return enabled
 
     @classmethod
-    def detect_should_enable(cls, recording_method):
+    def any_enabled(cls):
+        for m in RECORDING_METHODS:
+            _, enabled = cls._detect_should_enable(m)
+            if enabled:
+                return True
+        return False
+
+    @classmethod
+    def _detect_should_enable(cls, recording_method):
         if not recording_method:
             return ["no recording method is set", False]
 

appmap/_implementation/env.py

@@ -5,6 +5,8 @@
 from os import environ
 from pathlib import Path
 
+from .detect_enabled import DetectEnabled
+
 _cwd = Path.cwd()
 _bootenv = environ.copy()
 
@@ -34,6 +36,7 @@ def __init__(self, env=None, cwd=None):
         self._env = _bootenv.copy()
         if env:
             self._env.update(env)
+        self._enabled = DetectEnabled.any_enabled()
 
         self._root_dir = str(self._cwd) + "/"
         self._root_dir_len = len(self._root_dir)
@@ -66,20 +69,16 @@ def output_dir(self):
 
     @property
     def enabled(self):
-        return self.get("APPMAP", "false").lower() == "true"
+        return self._enabled
 
     @enabled.setter
     def enabled(self, value):
-        self.set("APPMAP", "true" if value else "false")
+        self._enabled = value
 
     @property
     def display_params(self):
         return self.get("APPMAP_DISPLAY_PARAMS", "true").lower() == "true"
 
-    @property
-    def record_all_requests(self):
-        return self.get("APPMAP_RECORD_REQUESTS", "false").lower() == "true"
-
     def _configure_logging(self):
         log_level = self.get("APPMAP_LOG_LEVEL", "warning").upper()
 

appmap/_implementation/importer.py

@@ -200,7 +200,11 @@ def instrument_functions(filterable):
         classes = get_classes(mod)
         logger.debug("  classes %s", classes)
         for c in classes:
-            instrument_functions(FilterableCls(c))
+            fc = FilterableCls(c)
+            if fc.fqname.startswith("appmap"):
+                logger.debug(f"  not instrumenting {fc.fqname}")
+                continue
+            instrument_functions(fc)
 
 
 def wrap_finder_function(fn, decorator):

appmap/flask.py

@@ -57,8 +57,7 @@ def init_app(self, app):
             # it may record requests but not remote (APPMAP=false)
             self.recorder = Recorder.get_current()
 
-        if Env.current.enabled:
-            # the remote recording routes are enabled only if APPMAP=true
+        if DetectEnabled.should_enable("remote"):
             app.add_url_rule(
                 self.record_url,
                 "appmap_record_get",

appmap/test/conftest.py

@@ -6,6 +6,7 @@
 
 import appmap._implementation
 from appmap._implementation import utils
+from appmap._implementation.detect_enabled import RECORDING_METHODS, DetectEnabled
 from appmap._implementation.env import Env
 from appmap._implementation.recorder import Recorder
 
@@ -55,6 +56,12 @@ def pytest_runtest_setup(item):
         appmap_enabled = mark_enabled.kwargs.get("appmap_enabled", "true")
         if isinstance(appmap_enabled, str):
             env["APPMAP"] = appmap_enabled
+        elif appmap_enabled is True:
+            env["APPMAP"] = "true"
+        elif appmap_enabled is False:
+            env["APPMAP"] = "false"
+        elif appmap_enabled is None:
+            env.pop("APPMAP", None)
 
     if mark_record_requests:
         appmap_record_requests = mark_record_requests.kwargs.get(
@@ -77,7 +84,7 @@ def git_directory_fixture(tmp_path_factory):
     (git_dir / "new_file").write_text("new_file")
 
     git = utils.git(cwd=git_dir)
-    git("init")
+    git("init --initial-branch main")
     git("config --local user.email test@test")
     git("config --local user.name Test")
     git("add README.metadata")

appmap/test/test_configuration.py

@@ -1,15 +1,13 @@
 """Test Configuration"""
 # pylint: disable=missing-function-docstring
 
-import sys
 from distutils.dir_util import copy_tree
 from pathlib import Path
 
 import pytest
 
 import appmap
 import appmap._implementation
-import appmap._implementation.env as impl_env
 from appmap._implementation.configuration import Config, ConfigFilter
 from appmap._implementation.env import Env
 from appmap._implementation.importer import Filterable, NullFilter
@@ -47,28 +45,21 @@ def test_reports_invalid():
     assert not Config().file_valid
 
 
+@pytest.mark.appmap_enabled(config="appmap-broken.yml", appmap_enabled=None)
 def test_is_disabled_when_unset():
     """Test that recording is disabled when APPMAP is unset"""
     assert Env.current.get("APPMAP", None) is None
 
     assert not appmap.enabled()
 
 
+@pytest.mark.appmap_enabled(config="appmap-broken.yml", appmap_enabled="false")
 def test_is_disabled_when_false():
     """Test that recording is disabled when APPMAP=false"""
     Env.current.set("APPMAP", "false")
     assert not appmap.enabled()
 
 
-@pytest.mark.appmap_enabled(appmap_enabled=None)
-def test_is_disabled_with_valid_config():
-    c = Config()
-    assert c.file_present
-    assert c.file_valid
-
-    assert not appmap.enabled()
-
-
 def test_config_not_found(caplog):
     appmap._implementation.initialize(
         env={  # pylint: disable=protected-access
@@ -86,15 +77,16 @@ def test_config_not_found(caplog):
     assert f'"{not_found}" is missing' in caplog.text
 
 
+@pytest.mark.appmap_enabled(appmap_enabled="false", config="notfound.yml")
 def test_config_no_message(caplog):
     """
     Messages about a missing config should only be logged when
     recording is enabled
     """
 
-    assert Config().name is None
     assert not appmap.enabled()
-    assert caplog.text is ""
+    assert Config().name is None
+    assert caplog.text == ""
 
 
 cf = lambda: ConfigFilter(NullFilter())
@@ -200,6 +192,7 @@ def test_created_if_missing_and_enabled(self, git, data_dir, monkeypatch):
         assert path.is_file()
 
     def test_not_created_if_missing_and_not_enabled(self, git, data_dir, monkeypatch):
+        monkeypatch.setenv("APPMAP", "false")
         repo_root = git.cwd
         copy_tree(data_dir / "config", str(repo_root))
         monkeypatch.chdir(repo_root)
@@ -208,7 +201,7 @@ def test_not_created_if_missing_and_not_enabled(self, git, data_dir, monkeypatch
         assert not path.is_file()
 
         # pylint: disable=protected-access
-        appmap._implementation.initialize(cwd=repo_root, env={"APPMAP": "false"})
+        appmap._implementation.initialize(cwd=repo_root)
 
         c = Config()
         assert not path.is_file()

appmap/test/test_flask.py

@@ -11,6 +11,11 @@
 from appmap.test.helpers import DictIncluding
 
 from .._implementation.metadata import Metadata
+
+# Make sure assertions in web_framework get rewritten (e.g. to show
+# diffs in generated appmaps)
+pytest.register_assert_rewrite("appmap.test.web_framework")
+
 from .web_framework import (  # pylint: disable=unused-import
     TestRecording,
     TestRecordRequests,

appmap/test/test_metadata.py

@@ -23,7 +23,7 @@ def test_git_metadata(git):
     assert git_md == DictIncluding(
         {
             "repository": "https://www.example.test/repo.git",
-            "branch": "master",
+            "branch": "main",
             "status": ["?? new_file"],
         }
     )
@@ -55,7 +55,7 @@ def test_tags(git):
     assert git_md == DictIncluding(
         {
             "repository": "https://www.example.test/repo.git",
-            "branch": "master",
+            "branch": "main",
             "tag": tag,
             "annotated_tag": atag,
             "commits_since_tag": 1,