Add Flask 2 support

appmap/__init__.py

@@ -5,6 +5,12 @@
 from ._implementation.labels import labels  # noqa: F401
 from ._implementation.recording import Recording  # noqa: F401
 
+try:
+    from . import django  # noqa: F401
+except ImportError:
+    # not using django
+    pass
+
 try:
     from . import flask  # noqa: F401
 except ImportError:

appmap/_implementation/flask.py

@@ -0,0 +1,39 @@
+""" remote_recording is a Flask app that can be mounted to expose the remote-recording endpoint. """
+import json
+
+from flask import Flask
+
+from . import generation
+from .recorder import Recorder
+from .web_framework import AppmapMiddleware
+
+remote_recording = Flask(__name__)
+
+
+@remote_recording.route("/record", methods=["GET"])
+def status():
+    if not AppmapMiddleware.should_record():
+        return "Appmap is disabled.", 404
+
+    return {"enabled": Recorder.get_current().get_enabled()}
+
+
+@remote_recording.route("/record", methods=["POST"])
+def start():
+    r = Recorder.get_current()
+    if r.get_enabled():
+        return "Recording is already in progress", 409
+
+    r.start_recording()
+    return "", 200
+
+
+@remote_recording.route("/record", methods=["DELETE"])
+def stop():
+    r = Recorder.get_current()
+    if not r.get_enabled():
+        return "No recording is in progress", 404
+
+    r.stop_recording()
+
+    return json.loads(generation.dump(r))

appmap/_implementation/web_framework.py

@@ -134,7 +134,8 @@ class AppmapMiddleware(ABC):
     def __init__(self):
         self.record_url = "/_appmap/record"
 
-    def should_record(self):
+    @staticmethod
+    def should_record():
         return DetectEnabled.should_enable("remote") or DetectEnabled.should_enable(
             "requests"
         )

appmap/flask.py

@@ -1,20 +1,19 @@
-import datetime
-import json
-import os.path
+import re
 import time
-from functools import wraps
 
 import flask
 import flask.cli
 import jinja2
 from flask import _app_ctx_stack, request
+from flask.cli import ScriptInfo
 from werkzeug.exceptions import BadRequest
-from werkzeug.routing import parse_rule
+from werkzeug.middleware.dispatcher import DispatcherMiddleware
 
-from appmap._implementation import generation, web_framework
+import appmap.wrapt as wrapt
 from appmap._implementation.detect_enabled import DetectEnabled
 from appmap._implementation.env import Env
 from appmap._implementation.event import HttpServerRequestEvent, HttpServerResponseEvent
+from appmap._implementation.flask import remote_recording
 from appmap._implementation.recorder import Recorder
 from appmap._implementation.web_framework import AppmapMiddleware
 from appmap._implementation.web_framework import TemplateHandler as BaseTemplateHandler
@@ -45,62 +44,40 @@ def request_params(req):
     return values_dict(params.lists())
 
 
+NP_PARAMS = re.compile(r"<Rule '(.*?)'")
+NP_PARAM_DELIMS = str.maketrans("<>", "{}")
+
+
 class AppmapFlask(AppmapMiddleware):
-    def __init__(self, app=None):
+    """
+    A Flask extension to add remote recording to an application.
+    Should be loaded by default, but can also be added manually.
+
+    For example:
+
+    ```
+    from appmap.flask import AppmapFlask
+
+    app = new Flask(__Name__)
+    AppmapFlask().init_app(app)
+    ```
+    """
+
+    def __init__(self):
         super().__init__()
-        self.app = app
-        if app is not None:
-            self.init_app(app)
 
     def init_app(self, app):
         if self.should_record():
-            # it may record requests but not remote (APPMAP=false)
             self.recorder = Recorder.get_current()
 
         if DetectEnabled.should_enable("remote"):
-            app.add_url_rule(
-                self.record_url,
-                "appmap_record_get",
-                view_func=self.record_get,
-                methods=["GET"],
-            )
-            app.add_url_rule(
-                self.record_url,
-                "appmap_record_post",
-                view_func=self.record_post,
-                methods=["POST"],
-            )
-            app.add_url_rule(
-                self.record_url,
-                "appmap_record_delete",
-                view_func=self.record_delete,
-                methods=["DELETE"],
+            app.wsgi_app = DispatcherMiddleware(
+                app.wsgi_app, {"/_appmap": remote_recording}
             )
 
         app.before_request(self.before_request)
         app.after_request(self.after_request)
 
-    def record_get(self):
-        if not self.should_record():
-            return "Appmap is disabled.", 404
-
-        return {"enabled": self.recorder.get_enabled()}
-
-    def record_post(self):
-        if self.recorder.get_enabled():
-            return "Recording is already in progress", 409
-
-        self.recorder.start_recording()
-        return "", 200
-
-    def record_delete(self):
-        if not self.recorder.get_enabled():
-            return "No recording is in progress", 404
-
-        self.recorder.stop_recording()
-
-        return json.loads(generation.dump(self.recorder))
-
     def before_request(self):
         if not self.should_record():
             return
@@ -112,16 +89,17 @@ def before_request(self):
     def before_request_main(self, rec, request):
         Metadata.add_framework("flask", flask.__version__)
         np = None
-        # See
-        # https://github.com/pallets/werkzeug/blob/2.0.0/src/werkzeug/routing.py#L213
-        # for a description of parse_rule.
         if request.url_rule:
-            np = "".join(
-                [
-                    f"{{{p}}}" if c else p
-                    for c, _, p in parse_rule(request.url_rule.rule)
-                ]
-            )
+            # Transform request.url to the expected normalized-path form. For example,
+            # "/post/<username>/<post_id>/summary" becomes "/post/{username}/{post_id}/summary".
+            # Notes:
+            #   * the value of `repr` of this rule begins with "<Rule '/post/<username>/<post_id>/summary'"
+            #   * the variable names in a rule can only contain alphanumerics:
+            #     * flask 1: https://github.com/pallets/werkzeug/blob/1dde4b1790f9c46b7122bb8225e6b48a5b22a615/src/werkzeug/routing.py#L143
+            #     * flask 2: https://github.com/pallets/werkzeug/blob/99f328cf2721e913bd8a3128a9cdd95ca97c334c/src/werkzeug/routing/rules.py#L56
+            r = repr(request.url_rule)
+            np = NP_PARAMS.findall(r)[0].translate(NP_PARAM_DELIMS)
+
         call_event = HttpServerRequestEvent(
             request_method=request.method,
             path_info=request.path,
@@ -174,18 +152,18 @@ class TemplateHandler(BaseTemplateHandler):
     pass
 
 
-def wrap_cli_fn(fn):
-    @wraps(fn)
-    def install_middleware(*args, **kwargs):
-        app = fn(*args, **kwargs)
-        if app:
-            appmap_flask = AppmapFlask()
-            appmap_flask.init_app(app)
-        return app
+def install_extension(wrapped, _, args, kwargs):
+    app = wrapped(*args, **kwargs)
+    if app:
+        AppmapFlask().init_app(app)
 
-    return install_middleware
+    return app
 
 
 if Env.current.enabled:
-    flask.cli.call_factory = wrap_cli_fn(flask.cli.call_factory)
-    flask.cli.locate_app = wrap_cli_fn(flask.cli.locate_app)
+    # ScriptInfo.load_app is the function that's used by the Flask cli to load an app, no matter how
+    # the app's module is specified (e.g. with the FLASK_APP env var, the `--app` flag, etc). Hook
+    # it so it installs our extension on the app.
+    ScriptInfo.load_app = wrapt.wrap_function_wrapper(
+        "flask.cli", "ScriptInfo.load_app", install_extension
+    )

appmap/test/data/django/app/settings.py

@@ -10,5 +10,3 @@
 # Must set ROOT_URLCONF else we get
 # AttributeError: 'Settings' object has no attribute 'ROOT_URLCONF'
 ROOT_URLCONF = "app.urls"
-
-MIDDLEWARE = ["appmap.django.Middleware"]

appmap/test/data/django/init/sitecustomize.py

@@ -0,0 +1 @@
+import appmap

appmap/test/data/flask/app.py

@@ -1,15 +1,16 @@
-"""Rudimentary Flask application for testing."""
+"""
+Rudimentary Flask application for testing.
+
+NB: This should not explicitly reference the `appmap` module in any way. Doing so invalidates
+testing of record-by-default.
+"""
 # pylint: disable=missing-function-docstring
 
 from flask import Flask, make_response
 from markupsafe import escape
 
-from appmap.flask import AppmapFlask
-
 app = Flask(__name__)
 
-appmap_flask = AppmapFlask(app)
-
 
 @app.route("/")
 def hello_world():

appmap/test/data/flask/init/sitecustomize.py

@@ -0,0 +1 @@
+import appmap

appmap/test/test_django.py

@@ -208,12 +208,15 @@ def test_middleware_reset(pytester, monkeypatch):
 class TestRecordRequestsDjango(TestRecordRequests):
     @staticmethod
     def server_start_thread(env_vars_str):
+        # Use appmap from our working copy, not the module installed by virtualenv. Add the init
+        # directory so the sitecustomize.py file it contains will be loaded on startup. This
+        # simulates a real installation.
         exec_cmd(
             """
-# use appmap from our working copy, not the module installed by virtualenv
-export PYTHONPATH=`pwd`
+export PYTHONPATH="$PWD"
 
 cd appmap/test/data/django/
+PYTHONPATH="$PYTHONPATH:$PWD/init"
 """
             + env_vars_str
             + """ APPMAP_OUTPUT_DIR=/tmp  python manage.py runserver 127.0.0.1:"""

appmap/test/test_flask.py

@@ -8,6 +8,7 @@
 import pytest
 
 from appmap._implementation.env import Env
+from appmap.flask import AppmapFlask
 from appmap.test.helpers import DictIncluding
 
 from .._implementation.metadata import Metadata
@@ -40,6 +41,11 @@ def flask_app(data_dir, monkeypatch):
     import app  # pylint: disable=import-error
 
     importlib.reload(app)
+
+    # Add the AppmapFlask extension to the app. This now happens automatically when a Flask app is
+    # started from the command line, but must be done manually otherwise.
+    AppmapFlask().init_app(app.app)
+
     return app.app
 
 
@@ -70,12 +76,15 @@ def test_template(app, events):
 class TestRecordRequestsFlask(TestRecordRequests):
     @staticmethod
     def server_start_thread(env_vars_str):
+        # Use appmap from our working copy, not the module installed by virtualenv. Add the init
+        # directory so the sitecustomize.py file it contains will be loaded on startup. This
+        # simulates a real installation.
         exec_cmd(
             """
-# use appmap from our working copy, not the module installed by virtualenv
-export PYTHONPATH=`pwd`
+export PYTHONPATH="$PWD"
 
 cd appmap/test/data/flask/
+PYTHONPATH="$PYTHONPATH:$PWD/init"
 """
             + env_vars_str
             + """ APPMAP_OUTPUT_DIR=/tmp FLASK_DEBUG=1 FLASK_APP=app.py flask run -p """

pyproject.toml

@@ -58,11 +58,6 @@ pyfakefs = "^4.3.2"
 pprintpp = ">=0.4.0"
 coverage = "^5.3"
 pytest-mock = "^3.5.1"
-flask = "^1.1.2"
-MarkupSafe = "2.0.1"
-jinja2 = ">=2.10.1,<3"
-itsdangerous = ">=0.24,<2"
-Werkzeug = ">=0.15,<2"
 SQLAlchemy = { version = "^1.4.11", python = "^3.7"}
 tox = "^3.22.0"
 tox-pyenv = "^1.1.0"

requirements-dev.txt

@@ -0,0 +1,7 @@
+#requirements-dev.txt
+pip
+poetry
+tox
+django
+flask
+pytest-django

requirements-flask1.txt

@@ -0,0 +1,5 @@
+flask == 1.1.2
+MarkupSafe == 2.0.1
+jinja2 >=2.10.1, <3
+itsdangerous >=0.24,<2
+Werkzeug >=0.15,<2

tox.ini

@@ -1,20 +1,27 @@
 [tox]
 isolated_build = true
-envlist = py37-django{2,3}, py3{8,9,10}-django{2,3,4}
+# The *-web environments test the latest versions of Django and Flask with the full test suite. For
+# older version of the web frameworks, just run the tests that are specific to them.
+envlist = py3{8,9,10}-web, py3{7,8,9,10}-flask1-django{2,3}
 
 [testenv]
+
 deps=
     poetry>=1.2.0
     pytest-django
-    django2: Django>=2.2,<3.0
-    django3: Django>=3.2,<4.0
-    django4: Django>=4.0,<5.0
+    web: Django >=4.0, <5.0
+    web: Flask >= 2
+    flask1: -rrequirements-flask1.txt
+    django2: Django >=2.2, <3.0
+    django3: Django >=3.2, <4.0
+
 
 allowlist_externals = env
 commands =
     # Turn off recording while installing. It's not necessary, and the warning messages that come
     # out of the agent confuse poetry.
     env APPMAP=false poetry install -v
-    django3: poetry run {posargs:pytest -v}
+    web: poetry run {posargs:pytest -vv}
+    flask1: poetry run pytest appmap/test/test_flask.py
     django2: poetry run pytest appmap/test/test_django.py
-    django4: poetry run pytest appmap/test/test_django.py
+    django3: poetry run pytest appmap/test/test_django.py