diff --git a/appmap/_implementation/__init__.py b/appmap/_implementation/__init__.py index 9911d333..b86831e3 100644 --- a/appmap/_implementation/__init__.py +++ b/appmap/_implementation/__init__.py @@ -1,6 +1,6 @@ from . import configuration from . import env as appmapenv -from . import event, importer, metadata, recorder +from . import event, importer, metadata, recorder, web_framework from .py_version_check import check_py_version @@ -12,6 +12,7 @@ def initialize(**kwargs): recorder.initialize() configuration.initialize() # needs to be initialized after recorder metadata.initialize() + web_framework.initialize() initialize() diff --git a/appmap/_implementation/configuration.py b/appmap/_implementation/configuration.py index 437d32e6..944fc893 100644 --- a/appmap/_implementation/configuration.py +++ b/appmap/_implementation/configuration.py @@ -218,7 +218,7 @@ def _load_config(self): Env.current.enabled = should_enable except ParserError: pass - logger.info("config: %s", self._config) + logger.debug("config: %s", self._config) return if not Env.current.enabled: diff --git a/appmap/_implementation/django.py b/appmap/_implementation/django.py new file mode 100644 index 00000000..da05af35 --- /dev/null +++ b/appmap/_implementation/django.py @@ -0,0 +1,34 @@ +""" +This file contains Django middleware that can be inserted into an app's stack to expose the remote +recording endpoint. + +It expects the importer to have verified that Django is available. +""" + +from http import HTTPStatus + +from django.http import HttpRequest, HttpResponse + +from . import remote_recording + + +class RemoteRecording: # pylint: disable=missing-class-docstring + def __init__(self, get_response): + super().__init__() + self.get_response = get_response + + def __call__(self, request: HttpRequest): + if request.path != "/_appmap/record": + return self.get_response(request) + + handlers = { + "GET": remote_recording.status, + "POST": remote_recording.start, + "DELETE": remote_recording.stop, + } + + def not_allowed(): + return "", HTTPStatus.METHOD_NOT_ALLOWED + + body, status = handlers.get(request.method, not_allowed)() + return HttpResponse(body, status=status, content_type="application/json") diff --git a/appmap/_implementation/env.py b/appmap/_implementation/env.py index f18a2636..8d49f558 100644 --- a/appmap/_implementation/env.py +++ b/appmap/_implementation/env.py @@ -80,7 +80,7 @@ def display_params(self): return self.get("APPMAP_DISPLAY_PARAMS", "true").lower() == "true" def _configure_logging(self): - log_level = self.get("APPMAP_LOG_LEVEL", "warning").upper() + log_level = self.get("APPMAP_LOG_LEVEL", "info").upper() log_config = self.get("APPMAP_LOG_CONFIG") log_stream = self.get("APPMAP_LOG_STREAM", "stderr") diff --git a/appmap/_implementation/event.py b/appmap/_implementation/event.py index b0bd128e..dc7a666b 100644 --- a/appmap/_implementation/event.py +++ b/appmap/_implementation/event.py @@ -1,3 +1,4 @@ +# pylint: disable=missing-module-docstring,missing-class-docstring,missing-function-docstring import inspect import logging import threading @@ -90,7 +91,7 @@ class Event: __slots__ = ["id", "event", "thread_id"] def __init__(self, event): - self.id = Recorder.get_current().next_event_id() + self.id = Recorder.next_event_id() self.event = event self.thread_id = _EventIds.get_thread_id() diff --git a/appmap/_implementation/flask.py b/appmap/_implementation/flask.py index 66932d6b..66a32901 100644 --- a/appmap/_implementation/flask.py +++ b/appmap/_implementation/flask.py @@ -1,39 +1,30 @@ -""" remote_recording is a Flask app that can be mounted to expose the remote-recording endpoint. """ -import json +""" +This file contains a Flask app that is mounted on /_appmap to expose the remote-recording endpoint +in a user's app. -from flask import Flask +It should only be imported if other code has already verified that Flask is available. +""" -from . import generation -from .recorder import Recorder -from .web_framework import AppmapMiddleware +from flask import Flask, Response -remote_recording = Flask(__name__) +from . import remote_recording +app = 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()} +@app.route("/record", methods=["GET"]) +def status(): + body, rrstatus = remote_recording.status() + return Response(body, status=rrstatus, mimetype="application/json") -@remote_recording.route("/record", methods=["POST"]) +@app.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 + body, rrstatus = remote_recording.start() + return Response(body, status=rrstatus, mimetype="application/json") -@remote_recording.route("/record", methods=["DELETE"]) +@app.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)) + body, rrstatus = remote_recording.stop() + return Response(body, status=rrstatus, mimetype="application/json") diff --git a/appmap/_implementation/importer.py b/appmap/_implementation/importer.py index 106658dc..af77de0d 100644 --- a/appmap/_implementation/importer.py +++ b/appmap/_implementation/importer.py @@ -187,7 +187,7 @@ def instrument_functions(filterable): if not cls.filter_chain.filter(filterable): return - logger.info(" looking for members of %s", filterable.obj) + logger.debug(" looking for members of %s", filterable.obj) functions = get_members(filterable.obj) logger.debug(" functions %s", functions) diff --git a/appmap/_implementation/instrument.py b/appmap/_implementation/instrument.py index a1673b60..4eb79b80 100644 --- a/appmap/_implementation/instrument.py +++ b/appmap/_implementation/instrument.py @@ -109,7 +109,7 @@ def call_instrumented(f, instance, args, kwargs): def instrument(filterable): """return an instrumented function""" - logger.info("hooking %s", filterable.fqname) + logger.debug("hooking %s", filterable.fqname) fn = filterable.obj make_call_event = event.CallEvent.make(fn, filterable.fntype) diff --git a/appmap/_implementation/recorder.py b/appmap/_implementation/recorder.py index fc4209d5..8c4c6bd3 100644 --- a/appmap/_implementation/recorder.py +++ b/appmap/_implementation/recorder.py @@ -7,6 +7,7 @@ logger = logging.getLogger(__name__) +# pylint: disable=global-statement _default_recorder = None @@ -22,10 +23,14 @@ class Recorder(ABC): def events(self): return self._events - @abstractmethod - def next_event_id(self): - self._next_event_id += 1 - return self._next_event_id + _next_event_id = 0 + _next_event_id_lock = threading.Lock() + + @classmethod + def next_event_id(cls): + with cls._next_event_id_lock: + cls._next_event_id += 1 + return cls._next_event_id # It might be nice to put @property on the getters here. The python maintainers have gone back # and forth on whether you should be able to combine @classmethod and @property. In 3.11, @@ -44,24 +49,40 @@ def set_current(cls, r): """ Set the recorder for the current thread. """ - appmap_tls()[cls._RECORDER_KEY] = r + tls = appmap_tls() + if r: + tls[cls._RECORDER_KEY] = r + else: + del tls[cls._RECORDER_KEY] + return r + @classmethod + def get_global(cls): + _, shared = cls._get_current() + return shared + + @classmethod + def new_global(cls): + global _default_recorder + _default_recorder = SharedRecorder() + return _default_recorder + @classmethod def get_enabled(cls): - return cls.get_current()._enabled + return cls.get_current()._enabled # pylint: disable=protected-access @classmethod def set_enabled(cls, e): - cls.get_current()._enabled = e + cls.get_current()._enabled = e # pylint: disable=protected-access @classmethod def start_recording(cls): - cls.get_current()._start_recording() + cls.get_current()._start_recording() # pylint: disable=protected-access @classmethod def stop_recording(cls): - return cls.get_current()._stop_recording() + return cls.get_current()._stop_recording() # pylint: disable=protected-access @classmethod def add_event(cls, event): @@ -70,28 +91,25 @@ def add_event(cls, event): one). """ perthread, shared = cls._get_current() - shared._add_event(event) + shared._add_event(event) # pylint: disable=protected-access if perthread: - perthread._add_event(event) + perthread._add_event(event) # pylint: disable=protected-access _RECORDER_KEY = "appmap_recorder" @classmethod def _get_current(cls): - global _default_recorder perthread = appmap_tls().get(cls._RECORDER_KEY, None) return [perthread, _default_recorder] def clear(self): self._events = [] - self._next_event_id = 0 def __init__(self, enabled=False): self._events = [] self._enabled = enabled self.start_tb = None - self._next_event_id = 0 @abstractmethod def _start_recording(self): @@ -122,8 +140,7 @@ def _initialize(): threads initializing the default recorder. If you find yourself wanting to do that, you should probably be using per-thread recording. """ - global _default_recorder - _default_recorder = SharedRecorder() + Recorder.new_global() class ThreadRecorder(Recorder): @@ -135,9 +152,8 @@ class ThreadRecorder(Recorder): def events(self): return super().events - def next_event_id(self): - return super().next_event_id() - + # They're not useless, because they're abtract with a default implementation + # pragma pylint: disable=useless-super-delegation def _start_recording(self): super()._start_recording() @@ -147,6 +163,8 @@ def _stop_recording(self): def _add_event(self, event): super()._add_event(event) + # pragma pylint: enable=useless-super-delegation + class SharedRecorder(Recorder): """ @@ -155,6 +173,14 @@ class SharedRecorder(Recorder): _lock = threading.Lock() + def __init__(self): + super().__init__() + Recorder._next_event_id = 0 + + def clear(self): + super().clear() + Recorder._next_event_id = 0 + @property def events(self): with self._lock: @@ -168,14 +194,10 @@ def _stop_recording(self): with self._lock: return super()._stop_recording() - def next_event_id(self): - with self._lock: - return super().next_event_id() - def _add_event(self, event): with self._lock: super()._add_event(event) def initialize(): - Recorder._initialize() + Recorder._initialize() # pylint: disable=protected-access diff --git a/appmap/_implementation/remote_recording.py b/appmap/_implementation/remote_recording.py new file mode 100644 index 00000000..516a577e --- /dev/null +++ b/appmap/_implementation/remote_recording.py @@ -0,0 +1,46 @@ +""" remote_recording is a Flask app that can be mounted to expose the remote-recording endpoint. """ +import json +from threading import Lock + +from . import generation +from .detect_enabled import DetectEnabled +from .recorder import Recorder + +# pylint: disable=global-statement +_enabled_lock = Lock() +_enabled = False + + +def status(): + if not DetectEnabled.should_enable("remote"): + return "Appmap is disabled.", 404 + + with _enabled_lock: + return json.dumps({"enabled": _enabled}), 200 + + +def start(): + global _enabled + with _enabled_lock: + if _enabled: + return "Recording is already in progress", 409 + + Recorder.new_global().start_recording() + _enabled = True + return "", 200 + + +def stop(): + global _enabled + with _enabled_lock: + if not _enabled: + return "No recording is in progress", 404 + r = Recorder.get_global() + r.stop_recording() + _enabled = False + return generation.dump(r), 200 + + +def initialize(): + global _enabled + _enabled = False diff --git a/appmap/_implementation/web_framework.py b/appmap/_implementation/web_framework.py index f063e86e..4b82d91e 100644 --- a/appmap/_implementation/web_framework.py +++ b/appmap/_implementation/web_framework.py @@ -6,15 +6,16 @@ import re import time from abc import ABC, abstractmethod +from contextvars import ContextVar from hashlib import sha256 from tempfile import NamedTemporaryFile -from appmap._implementation import generation -from appmap._implementation.detect_enabled import DetectEnabled -from appmap._implementation.env import Env -from appmap._implementation.event import Event, ReturnEvent, _EventIds, describe_value -from appmap._implementation.recorder import Recorder, ThreadRecorder -from appmap._implementation.utils import root_relative_path, scenario_filename +from . import generation, remote_recording +from .detect_enabled import DetectEnabled +from .env import Env +from .event import Event, ReturnEvent, describe_value +from .recorder import Recorder, ThreadRecorder +from .utils import root_relative_path, scenario_filename class TemplateEvent(Event): # pylint: disable=too-few-public-methods @@ -130,6 +131,9 @@ def create_appmap_file( headers["AppMap-File-Name"] = os.path.abspath(appmap_file_path) + APPMAP_SUFFIX +request_recorder = ContextVar("appmap_request_recorder") + + class AppmapMiddleware(ABC): def __init__(self): self.record_url = "/_appmap/record" @@ -140,7 +144,7 @@ def should_record(): "requests" ) - def before_request_hook(self, request, request_path, recording_is_running): + def before_request_hook(self, request, request_path): if request_path == self.record_url: return None, None, None @@ -152,15 +156,9 @@ def before_request_hook(self, request, request_path, recording_is_running): rec = ThreadRecorder() Recorder.set_current(rec) rec.start_recording() - # Each time an event is added for a thread_id it's also - # added to the global Recorder(). So don't add the event - # to the global Recorder() explicitly because that would - # add the event in it twice. - elif DetectEnabled.should_enable("remote") or recording_is_running: - # b) APPMAP=true, or - # c) remote, enabled by POST to /_appmap/record, which set - # recording_is_running - rec = Recorder.get_current() + request_recorder.set(rec) + elif DetectEnabled.should_enable("remote"): + rec = Recorder.get_global() if rec and rec.get_enabled(): start, call_event_id = self.before_request_main(rec, request) @@ -173,9 +171,7 @@ def before_request_main(self, rec): def after_request_hook( self, - request, request_path, - recording_is_running, request_method, request_base_url, response, @@ -188,14 +184,9 @@ def after_request_hook( if DetectEnabled.should_enable("requests"): # a) requests - rec = Recorder.get_current() - # Each time an event is added for a thread_id it's also - # added to the global Recorder(). So don't add the event - # to the global Recorder() explicitly because that would - # add the event in it twice. + rec = request_recorder.get() try: - if rec.get_enabled(): - self.after_request_main(rec, response, start, call_event_id) + self.after_request_main(rec, response, start, call_event_id) output_dir = Env.current.output_dir / "requests" create_appmap_file( @@ -209,11 +200,11 @@ def after_request_hook( ) finally: rec.stop_recording() - elif DetectEnabled.should_enable("remote") or recording_is_running: - # b) APPMAP=true, or - # c) remote, enabled by POST to /_appmap/record, which set - # recording_is_running - rec = Recorder.get_current() + Recorder.set_current(None) + request_recorder.set(None) + + elif DetectEnabled.should_enable("remote"): + rec = Recorder.get_global() if rec.get_enabled(): self.after_request_main(rec, response, start, call_event_id) @@ -222,3 +213,7 @@ def after_request_hook( @abstractmethod def after_request_main(self, rec, response, start, call_event_id): pass + + +def initialize(): + remote_recording.initialize() diff --git a/appmap/django.py b/appmap/django.py index 4e640a65..4171a445 100644 --- a/appmap/django.py +++ b/appmap/django.py @@ -16,29 +16,26 @@ from django.db.backends.signals import connection_created from django.db.backends.utils import CursorDebugWrapper from django.dispatch import receiver -from django.http import HttpResponse, HttpResponseBadRequest, JsonResponse from django.template import Template from django.urls import get_resolver, resolve from django.urls.exceptions import Resolver404 from django.urls.resolvers import _route_to_regex -from appmap._implementation import generation, recording, web_framework -from appmap._implementation.env import Env -from appmap._implementation.event import ( +from appmap._implementation.detect_enabled import DetectEnabled + +from ._implementation.event import ( ExceptionEvent, HttpServerRequestEvent, HttpServerResponseEvent, ReturnEvent, SqlEvent, - _EventIds, ) -from appmap._implementation.instrument import is_instrumentation_disabled -from appmap._implementation.recorder import Recorder -from appmap._implementation.web_framework import AppmapMiddleware -from appmap._implementation.web_framework import TemplateHandler as BaseTemplateHandler - +from ._implementation.instrument import is_instrumentation_disabled from ._implementation.metadata import Metadata +from ._implementation.recorder import Recorder from ._implementation.utils import patch_class, values_dict +from ._implementation.web_framework import AppmapMiddleware +from ._implementation.web_framework import TemplateHandler as BaseTemplateHandler def parse_pg_version(version): @@ -233,12 +230,7 @@ def __call__(self, request): if not self.should_record(): return self.get_response(request) - if request.path_info == self.record_url: - return self.recording(request) - - rec, start, call_event_id = self.before_request_hook( - request, request.path_info, self.recorder.get_enabled() - ) + rec, start, call_event_id = self.before_request_hook(request, request.path_info) try: response = self.get_response(request) @@ -254,9 +246,7 @@ def __call__(self, request): raise self.after_request_hook( - request, request.path_info, - self.recorder.get_enabled(), request.method, request.build_absolute_uri(), response, @@ -302,32 +292,14 @@ def after_request_main(self, rec, response, start, call_event_id): ) rec.add_event(return_event) - def recording(self, request): - """Handle recording requests.""" - if request.method == "GET": - return JsonResponse({"enabled": self.recorder.get_enabled()}) - if request.method == "POST": - if self.recorder.get_enabled(): - return HttpResponse("Recording is already in progress", status=409) - self.recorder.start_recording() - return HttpResponse() - - if request.method == "DELETE": - if not self.recorder.get_enabled(): - return HttpResponse("No recording is in progress", status=404) - - self.recorder.stop_recording() - return HttpResponse( - generation.dump(self.recorder), content_type="application/json" - ) - - return HttpResponseBadRequest() - def inject_middleware(): """Make sure AppMap middleware is added to the stack""" if "appmap.django.Middleware" not in settings.MIDDLEWARE: - settings.MIDDLEWARE.insert(0, "appmap.django.Middleware") + new_middleware = ["appmap.django.Middleware"] + if DetectEnabled.should_enable("remote"): + new_middleware.insert(0, "appmap._implementation.django.RemoteRecording") + settings.MIDDLEWARE[0:0] = new_middleware original_load_middleware = BaseHandler.load_middleware diff --git a/appmap/flask.py b/appmap/flask.py index 7a141ecc..cf818d9e 100644 --- a/appmap/flask.py +++ b/appmap/flask.py @@ -4,7 +4,7 @@ import flask import flask.cli import jinja2 -from flask import _app_ctx_stack, request +from flask import g, request from flask.cli import ScriptInfo from werkzeug.exceptions import BadRequest from werkzeug.middleware.dispatcher import DispatcherMiddleware @@ -13,8 +13,7 @@ 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.flask import app as remote_recording_app from appmap._implementation.web_framework import AppmapMiddleware from appmap._implementation.web_framework import TemplateHandler as BaseTemplateHandler @@ -63,16 +62,10 @@ class AppmapFlask(AppmapMiddleware): ``` """ - def __init__(self): - super().__init__() - def init_app(self, app): - if self.should_record(): - self.recorder = Recorder.get_current() - if DetectEnabled.should_enable("remote"): app.wsgi_app = DispatcherMiddleware( - app.wsgi_app, {"/_appmap": remote_recording} + app.wsgi_app, {"/_appmap": remote_recording_app} ) app.before_request(self.before_request) @@ -82,14 +75,13 @@ def before_request(self): if not self.should_record(): return - rec, start, call_event_id = self.before_request_hook( - request, request.path, self.recorder.get_enabled() - ) + self.before_request_hook(request, request.path) def before_request_main(self, rec, request): Metadata.add_framework("flask", flask.__version__) np = None if request.url_rule: + # pragma pylint: disable=line-too-long # Transform request.url to the expected normalized-path form. For example, # "/post///summary" becomes "/post/{username}/{post_id}/summary". # Notes: @@ -97,6 +89,7 @@ def before_request_main(self, rec, request): # * 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 + # pragma pylint: enable=line-too-long r = repr(request.url_rule) np = NP_PARAMS.findall(r)[0].translate(NP_PARAM_DELIMS) @@ -110,10 +103,10 @@ def before_request_main(self, rec, request): ) rec.add_event(call_event) - appctx = _app_ctx_stack.top - appctx.appmap_request_event = call_event - appctx.appmap_request_start = time.monotonic() - + # Flask 2 removed the suggestion to use _app_ctx_stack.top, and instead says extensions + # should use g with a private property. + g._appmap_request_event = call_event # pylint: disable=protected-access + g._appmap_request_start = time.monotonic() # pylint: disable=protected-access return None, None def after_request(self, response): @@ -121,9 +114,7 @@ def after_request(self, response): return response return self.after_request_hook( - request, request.path, - self.recorder.get_enabled(), request.method, request.base_url, response, @@ -133,10 +124,9 @@ def after_request(self, response): ) def after_request_main(self, rec, response, start, call_event_id): - appctx = _app_ctx_stack.top - parent_id = appctx.appmap_request_event.id - duration = time.monotonic() - appctx.appmap_request_start - + parent_id = g._appmap_request_event.id # pylint: disable=protected-access + start = g._appmap_request_start # pylint: disable=protected-access + duration = time.monotonic() - start return_event = HttpServerResponseEvent( parent_id=parent_id, elapsed=duration, diff --git a/appmap/test/test_django.py b/appmap/test/test_django.py index b4c65c3c..31f487cd 100644 --- a/appmap/test/test_django.py +++ b/appmap/test/test_django.py @@ -1,9 +1,10 @@ # flake8: noqa: E402 -# pylint: disable=unused-import, redefined-outer-name, missing-function-docstring +# pylint: disable=missing-function-docstring,redefined-outer-name import json import sys from pathlib import Path +from threading import Thread import django import django.conf @@ -19,23 +20,17 @@ import appmap.django # noqa: F401 from appmap.test.helpers import DictIncluding +from .._implementation.metadata import Metadata +from .web_framework import TestRecordRequests, exec_cmd, wait_until_port_is + sys.path += [str(Path(__file__).parent / "data" / "django")] -import app -from .._implementation.metadata import Metadata +# Import app just for the side-effects. It must happen after sys.path has been modified. +import app # pyright: ignore pylint: disable=import-error, unused-import,wrong-import-order # 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 threading import Thread - -from .web_framework import ( - TestRecording, - TestRecordRequests, - TestRequestCapture, - exec_cmd, - wait_until_port_is, -) @pytest.mark.django_db @@ -235,7 +230,11 @@ def server_start(env_vars_str): @staticmethod def server_stop(): exec_cmd( - "ps -ef | grep -i 'manage.py runserver' | grep -v grep | awk '{ print $2 }' | xargs kill -9" + "ps -ef" + + "| grep -i 'manage.py runserver'" + + "| grep -v grep" + + "| awk '{ print $2 }'" + + "| xargs kill -9" ) wait_until_port_is("127.0.0.1", TestRecordRequests.server_port, "closed") diff --git a/appmap/test/test_flask.py b/appmap/test/test_flask.py index bd8fe3d4..f03fdbd9 100644 --- a/appmap/test/test_flask.py +++ b/appmap/test/test_flask.py @@ -12,19 +12,16 @@ 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, - TestRequestCapture, exec_cmd, wait_until_port_is, ) +# Make sure assertions in web_framework get rewritten (e.g. to show +# diffs in generated appmaps) +pytest.register_assert_rewrite("appmap.test.web_framework") + @pytest.fixture(name="client") def flask_client(app): @@ -38,7 +35,7 @@ def flask_app(data_dir, monkeypatch): Env.current.set("APPMAP_CONFIG", data_dir / "flask" / "appmap.yml") - import app # pylint: disable=import-error + import app # pyright: ignore pylint: disable=import-error,import-outside-toplevel importlib.reload(app) @@ -103,7 +100,11 @@ def server_start(env_vars_str): @staticmethod def server_stop(): exec_cmd( - "ps -ef | grep -i 'flask run' | grep -v grep | awk '{ print $2 }' | xargs kill -9" + "ps -ef" + + "| grep -i 'flask run'" + + "| grep -v grep" + + "| awk '{ print $2 }'" + + "| xargs kill -9" ) wait_until_port_is("127.0.0.1", TestRecordRequests.server_port, "closed") diff --git a/appmap/test/test_recording.py b/appmap/test/test_recording.py index da379685..cbc6a4f3 100644 --- a/appmap/test/test_recording.py +++ b/appmap/test/test_recording.py @@ -108,8 +108,6 @@ def test_can_deepcopy_function(self): modfunc, ) - from appmap.wrapt import FunctionWrapper - rec = appmap.Recording() with rec: f1 = deepcopy(modfunc) @@ -174,5 +172,4 @@ def add_event(name): for n in range(thread_count): events = recorders[f"thread{n}"].events assert len(events) == 1 - assert events[0].id == 1 assert events[0].event["name"] == f"thread{n}" diff --git a/appmap/test/web_framework.py b/appmap/test/web_framework.py index 14310f2d..7bcc8fc8 100644 --- a/appmap/test/web_framework.py +++ b/appmap/test/web_framework.py @@ -244,8 +244,9 @@ class TestRecording: def test_appmap_disabled(client, monkeypatch): # since APPMAP records by default, disable it explicitly monkeypatch.setenv("APPMAP", "false") - appmap._implementation.initialize() + appmap._implementation.initialize() # pylint: disable=protected-access assert not appmap.enabled() + assert not DetectEnabled.should_enable("remote") res = client.get("/_appmap/record") assert res.status_code == 404 @@ -279,8 +280,7 @@ def test_can_only_enable_once(client): @staticmethod @pytest.mark.appmap_enabled - def test_can_record(data_dir, client, monkeypatch): - monkeypatch.setenv("APPMAP_RECORD_REQUESTS", "false") + def test_can_record(data_dir, client): res = client.post("/_appmap/record") assert res.status_code == 200 @@ -297,7 +297,7 @@ def test_can_record(data_dir, client, monkeypatch): generated_appmap = normalize_appmap(data) expected_path = data_dir / "remote.appmap.json" - with open(expected_path) as expected: + with open(expected_path, encoding="utf-8") as expected: expected_appmap = json.load(expected) assert ( @@ -316,7 +316,7 @@ def exec_cmd(command): stdin=subprocess.PIPE, stderr=subprocess.STDOUT, ) - output, errors = p.communicate() + output, _ = p.communicate() return output.decode() @@ -326,7 +326,7 @@ def port_state(address, port): try: s.connect((address, port)) ret = "open" - except: + except Exception: # pylint: disable=broad-except ret = "closed" s.close() return ret @@ -353,7 +353,7 @@ class TestRecordRequests: server_port = 8000 @abstractmethod - def server_start(): + def server_start(env_vars_str): """ Start the webserver in the background. Don't block execution. """ @@ -405,7 +405,7 @@ def record_request( for future in concurrent.futures.as_completed(future_to_request_number): try: response = future.result() - except Exception as e: + except Exception: # pylint: disable=broad-except traceback.print_exc() assert response.status_code == 200 @@ -433,7 +433,7 @@ def record_request( == "http_127_0_0_1_8000_test.appmap.json" ) - with open(appmap_file_name) as f: + with open(appmap_file_name, encoding="utf-8") as f: appmap = json.loads(f.read()) # Every event should come from the same thread diff --git a/pylintrc b/pylintrc index 1232389e..ae737ef2 100644 --- a/pylintrc +++ b/pylintrc @@ -6,7 +6,7 @@ extension-pkg-whitelist= # Specify a score threshold to be exceeded before program exits with error. -fail-under=7.6 +fail-under=7.9 # Add files or directories to the blacklist. They should be base names, not # paths. @@ -60,7 +60,7 @@ confidence= # --enable=similarities". If you want to run only the classes checker, but have # no Warning level messages displayed, use "--disable=all --enable=classes # --disable=W". -disable= +disable=consider-using-fstring, invalid-name, missing-function-docstring, missing-class-docstring, missing-module-docstring, consider-using-f-string # Enable the message, report, category or checker with the given id(s). You can # either give multiple identifier separated by comma (,) or put this option diff --git a/tox.ini b/tox.ini index 516c0f07..868415b5 100644 --- a/tox.ini +++ b/tox.ini @@ -20,8 +20,8 @@ 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 - py310-web: poetry run pylint appmap + env APPMAP_LOG_LEVEL=warning APPMAP=false poetry install -v + py310-web: poetry run pylint -j 0 appmap web: poetry run {posargs:pytest -vv} flask1: poetry run pytest appmap/test/test_flask.py django2: poetry run pytest appmap/test/test_django.py