diff --git a/appmap/__init__.py b/appmap/__init__.py index 5f4823fd..fcc294d9 100644 --- a/appmap/__init__.py +++ b/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: diff --git a/appmap/_implementation/flask.py b/appmap/_implementation/flask.py new file mode 100644 index 00000000..66932d6b --- /dev/null +++ b/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)) diff --git a/appmap/_implementation/web_framework.py b/appmap/_implementation/web_framework.py index 3cfe6a8c..f063e86e 100644 --- a/appmap/_implementation/web_framework.py +++ b/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" ) diff --git a/appmap/flask.py b/appmap/flask.py index 5be7c13d..7a141ecc 100644 --- a/appmap/flask.py +++ b/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"", "{}") + + 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///summary" becomes "/post/{username}/{post_id}/summary". + # Notes: + # * the value of `repr` of this rule begins with "//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 + ) diff --git a/appmap/test/data/django/app/settings.py b/appmap/test/data/django/app/settings.py index 047ec3f0..90418e3e 100644 --- a/appmap/test/data/django/app/settings.py +++ b/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"] diff --git a/appmap/test/data/django/init/sitecustomize.py b/appmap/test/data/django/init/sitecustomize.py new file mode 100644 index 00000000..d1fe4fec --- /dev/null +++ b/appmap/test/data/django/init/sitecustomize.py @@ -0,0 +1 @@ +import appmap diff --git a/appmap/test/data/flask/app.py b/appmap/test/data/flask/app.py index 93b75aa4..7d326f8f 100644 --- a/appmap/test/data/flask/app.py +++ b/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(): diff --git a/appmap/test/data/flask/init/sitecustomize.py b/appmap/test/data/flask/init/sitecustomize.py new file mode 100644 index 00000000..d1fe4fec --- /dev/null +++ b/appmap/test/data/flask/init/sitecustomize.py @@ -0,0 +1 @@ +import appmap diff --git a/appmap/test/test_django.py b/appmap/test/test_django.py index 537a9a55..b4c65c3c 100644 --- a/appmap/test/test_django.py +++ b/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:""" diff --git a/appmap/test/test_flask.py b/appmap/test/test_flask.py index 357e7e26..bd8fe3d4 100644 --- a/appmap/test/test_flask.py +++ b/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 """ diff --git a/pyproject.toml b/pyproject.toml index b44bf3cf..5d01cc5f 100644 --- a/pyproject.toml +++ b/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" diff --git a/requirements-dev.txt b/requirements-dev.txt new file mode 100644 index 00000000..a4c6a1f2 --- /dev/null +++ b/requirements-dev.txt @@ -0,0 +1,7 @@ +#requirements-dev.txt +pip +poetry +tox +django +flask +pytest-django \ No newline at end of file diff --git a/requirements-flask1.txt b/requirements-flask1.txt new file mode 100644 index 00000000..11c5ae2b --- /dev/null +++ b/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 \ No newline at end of file diff --git a/tox.ini b/tox.ini index 6286d777..90b55434 100644 --- a/tox.ini +++ b/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