feat: support Flask 2

Fix up Flask support so it's compatible with v2. Also fixes initial
configuration, so a Flask app will be recorded by default.

Author: apotterri

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,8 +134,11 @@ class AppmapMiddleware(ABC):
     def __init__(self):
         self.record_url = "/_appmap/record"
 
-    def should_record(self):
-        return DetectEnabled.should_enable("remote") or DetectEnabled.should_enable("requests")
+    @staticmethod
+    def should_record():
+        return DetectEnabled.should_enable("remote") or DetectEnabled.should_enable(
+            "requests"
+        )
 
     def before_request_hook(self, request, request_path, recording_is_running):
         if request_path == self.record_url:

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.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
@@ -50,61 +49,35 @@ def request_params(req):
 
 
 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
@@ -182,18 +155,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/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_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 """

requirements-dev.txt

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