Repository: scoutapp/scout_apm_python
Pull request #698 (2021-11-05 19:04:17)

Instrument flask for error monitoring.


Patch:
diff --git a/CHANGELOG.md b/CHANGELOG.md
index 141df5a..731dd33 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -8,8 +8,12 @@
 - Run CI tests once per day to identify issues quicker.
 - Set Scout core agent default version to v1.4.0.
   ([Issue 700](https://github.com/scoutapp/scout_apm_python/issues/700))
+- Instrument Flask for Error Monitoring.
 
 ### Fixed
+- Properly handle querystring parameters in error monitoring service by
+  passing them as a list of tuples rather than a dictionary. When there
+  were multiple params with the same key, it was keeping the last value.
 
 ## [2.23.3] 2021-11-04
 
diff --git a/src/scout_apm/core/web_requests.py b/src/scout_apm/core/web_requests.py
index e4a6c31..7fb91f7 100644
--- a/src/scout_apm/core/web_requests.py
+++ b/src/scout_apm/core/web_requests.py
@@ -33,6 +33,7 @@ FILTER_PARAMETERS = frozenset(
         "protected",
         "salt",
         "secret",
+        "secret_key",
         "ssn",
         "stripetoken",
         "token",
diff --git a/src/scout_apm/django/apps.py b/src/scout_apm/django/apps.py
index 5568dbd..9525a1b 100644
--- a/src/scout_apm/django/apps.py
+++ b/src/scout_apm/django/apps.py
@@ -63,7 +63,7 @@ class ScoutApmDjangoConfig(AppConfig):
             sys.exc_info(),
             request_components=get_request_components(request),
             request_path=request.path,
-            request_params=request.GET.dict(),
+            request_params=[(k, v) for k, vs in request.GET.lists() for v in vs],
             session=dict(request.session.items())
             if hasattr(request, "session")
             else None,
diff --git a/src/scout_apm/flask/__init__.py b/src/scout_apm/flask/__init__.py
index ad4dc0e..2007630 100644
--- a/src/scout_apm/flask/__init__.py
+++ b/src/scout_apm/flask/__init__.py
@@ -1,14 +1,20 @@
 # coding=utf-8
 from __future__ import absolute_import, division, print_function, unicode_literals
 
+import logging
+import sys
+
 import wrapt
 from flask import current_app
-from flask.globals import request
+from flask.globals import request, session
 
 import scout_apm.core
 from scout_apm.core.config import scout_config
+from scout_apm.core.error import ErrorMonitor
 from scout_apm.core.tracked_request import TrackedRequest
-from scout_apm.core.web_requests import werkzeug_track_request_data
+from scout_apm.core.web_requests import RequestComponents, werkzeug_track_request_data
+
+logger = logging.getLogger(__name__)
 
 
 class ScoutApm(object):
@@ -35,11 +41,10 @@ class ScoutApm(object):
         if request.routing_exception is not None:
             return wrapped(*args, **kwargs)
 
-        rule = request.url_rule
-        view_func = instance.view_functions[rule.endpoint]
-
-        name = view_func.__module__ + "." + view_func.__name__
-        operation = "Controller/" + name
+        request_components = get_request_components(self.app, request)
+        operation = "Controller/{}.{}".format(
+            request_components.module, request_components.controller
+        )
 
         tracked_request = TrackedRequest.instance()
         tracked_request.is_real_request = True
@@ -56,6 +61,17 @@ class ScoutApm(object):
                 response = wrapped(*args, **kwargs)
             except Exception as exc:
                 tracked_request.tag("error", "true")
+                if scout_config.value("errors_enabled"):
+                    ErrorMonitor.send(
+                        sys.exc_info(),
+                        request_components=get_request_components(self.app, request),
+                        request_path=request.path,
+                        request_params=[
+                            (k, v) for k, vs in request.args.lists() for v in vs
+                        ],
+                        session=dict(session.items()),
+                        environment=self.app.config,
+                    )
                 raise exc
             else:
                 if 500 <= response.status_code <= 599:
@@ -92,3 +108,13 @@ class ScoutApm(object):
                 clean_name = name.replace("SCOUT_", "").lower()
                 configs[clean_name] = value
         scout_config.set(**configs)
+
+
+def get_request_components(app, request):
+    view_func = app.view_functions[request.endpoint]
+    request_components = RequestComponents(
+        module=view_func.__module__,
+        controller=view_func.__name__,
+        action=request.method,
+    )
+    return request_components


Test patch:
diff --git a/tests/integration/django_app.py b/tests/integration/django_app.py
index 9fe5353..6d81c95 100644
--- a/tests/integration/django_app.py
+++ b/tests/integration/django_app.py
@@ -82,6 +82,11 @@ def hello(request):
     return HttpResponse("Hello World!")
 
 
+def set_session(request):
+    request.session["session_var"] = 1
+    return HttpResponse("Set session")
+
+
 def crash(request):
     raise ValueError("BØØM!")  # non-ASCII
 
@@ -241,6 +246,7 @@ def urlpatterns():
             path("", home),
             path("hello/", hello),
             path("crash/", crash),
+            path("set-session/", set_session),
             path("return-error/", return_error),
             path("cbv/", CbvView.as_view()),
             path("get-username/", get_username),
@@ -263,6 +269,7 @@ def urlpatterns():
             url(r"^$", home),
             url(r"^hello/$", hello),
             url(r"^crash/$", crash),
+            url(r"^set-session/$", set_session),
             url(r"^return-error/$", return_error),
             url(r"^cbv/$", CbvView.as_view()),
             url(r"^get-username/$", get_username),
diff --git a/tests/integration/test_django.py b/tests/integration/test_django.py
index 0f30a56..644e866 100644
--- a/tests/integration/test_django.py
+++ b/tests/integration/test_django.py
@@ -254,6 +254,37 @@ def test_server_error_error_monitor(tracked_requests, error_monitor_errors):
     error = error_monitor_errors[0]
     assert error["exception_class"] == "ValueError"
     assert error["message"] == "BØØM!"
+    assert error["request_uri"] == "/crash/"
+    assert error["request_session"] is None
+    assert error["request_params"] is None
+    assert error["request_components"] == {
+        "module": "tests.integration.django_app",
+        "controller": "crash",
+        "action": "GET",
+    }
+
+
+def test_server_error_error_monitor_with_session(
+    tracked_requests, error_monitor_errors
+):
+    with app_with_scout(DEBUG_PROPAGATE_EXCEPTIONS=False) as app:
+        test_app = TestApp(app)
+        test_app.get("/set-session/")
+        test_app.get("/crash/", expect_errors=True)
+
+    assert len(error_monitor_errors) == 1
+    error = error_monitor_errors[0]
+    assert error["request_session"] == {"session_var": "1"}
+
+
+def test_server_error_error_monitor_with_params(tracked_requests, error_monitor_errors):
+    with app_with_scout(DEBUG_PROPAGATE_EXCEPTIONS=False) as app:
+        test_app = TestApp(app)
+        test_app.get("/crash/?spam[]=eggs&spam[]=false", expect_errors=True)
+
+    assert len(error_monitor_errors) == 1
+    error = error_monitor_errors[0]
+    assert error["request_params"] == [("spam[]", "eggs"), ("spam[]", "false")]
 
 
 def test_return_error(tracked_requests):
@@ -699,7 +730,7 @@ def test_error_monitor_sanitized_environment(tracked_requests, error_monitor_err
     assert response.status_int == 500
     assert len(error_monitor_errors) == 1
     error = error_monitor_errors[0]
-    assert error["environment"]["SECRET_KEY"] == "********************"
+    assert error["environment"]["SECRET_KEY"] == "[FILTERED]"
     assert error["environment"]["DATABASES"]["default"]["PASSWORD"] == "[FILTERED]"
 
 
diff --git a/tests/integration/test_flask.py b/tests/integration/test_flask.py
index 47848df..f8f8a67 100644
--- a/tests/integration/test_flask.py
+++ b/tests/integration/test_flask.py
@@ -45,6 +45,11 @@ def app_with_scout(config=None):
             return "Hello Options!"
         return "Hello World!"
 
+    @app.route("/set-session/")
+    def set_session():
+        flask.session["session_var"] = 1
+        return "Set session"
+
     @app.route("/crash/")
     def crash():
         raise ValueError("BØØM!")  # non-ASCII
@@ -56,6 +61,7 @@ def app_with_scout(config=None):
     # Setup according to https://docs.scoutapm.com/#flask
     ScoutApm(app)
     app.config.update(config)
+    app.secret_key = "123"
 
     try:
         yield app
@@ -239,3 +245,86 @@ def test_no_monitor(tracked_requests):
 
     assert response.status_int == 200
     assert tracked_requests == []
+
+
+def test_server_error_error_monitor(tracked_requests, error_monitor_errors):
+    with app_with_scout(config={"PROPAGATE_EXCEPTIONS": False}) as app:
+        TestApp(app).get("/crash/", expect_errors=True)
+
+    assert len(error_monitor_errors) == 1
+    error = error_monitor_errors[0]
+    assert error["exception_class"] == "ValueError"
+    assert error["message"] == "BØØM!"
+    assert error["request_uri"] == "/crash/"
+    assert error["request_session"] is None
+    assert error["request_params"] is None
+    assert error["request_components"] == {
+        "module": "tests.integration.test_flask",
+        "controller": "crash",
+        "action": "GET",
+    }
+
+
+def test_server_error_error_monitor_with_session(
+    tracked_requests, error_monitor_errors
+):
+    with app_with_scout(config={"PROPAGATE_EXCEPTIONS": False}) as app:
+        test_app = TestApp(app)
+        test_app.get("/set-session/")
+        test_app.get("/crash/", expect_errors=True)
+
+    assert len(error_monitor_errors) == 1
+    error = error_monitor_errors[0]
+    assert error["request_session"] == {"session_var": "1"}
+
+
+def test_server_error_error_monitor_with_params(tracked_requests, error_monitor_errors):
+    with app_with_scout(config={"PROPAGATE_EXCEPTIONS": False}) as app:
+        test_app = TestApp(app)
+        test_app.get("/crash/?spam[]=eggs&spam[]=false", expect_errors=True)
+
+    assert len(error_monitor_errors) == 1
+    error = error_monitor_errors[0]
+    assert error["request_params"] == [("spam[]", "eggs"), ("spam[]", "false")]
+
+
+def test_return_error_error_monitor(tracked_requests, error_monitor_errors):
+    with app_with_scout() as app:
+        TestApp(app).get("/return-error/", expect_errors=True)
+
+    assert len(error_monitor_errors) == 0
+
+
+def test_no_monitor_server_error(tracked_requests):
+    with app_with_scout(
+        config={"SCOUT_MONITOR": False, "PROPAGATE_EXCEPTIONS": False}
+    ) as app:
+        response = TestApp(app).get("/crash/", expect_errors=True)
+
+    assert response.status_int == 500
+    assert tracked_requests == []
+
+
+def test_no_error_monitoring_server_error(tracked_requests, error_monitor_errors):
+    with app_with_scout(
+        config={"SCOUT_ERRORS_ENABLED": False, "PROPAGATE_EXCEPTIONS": False}
+    ) as app:
+        response = TestApp(app).get("/crash/", expect_errors=True)
+
+    assert response.status_int == 500
+    assert error_monitor_errors == []
+
+
+def test_error_monitor_sanitized_environment(tracked_requests, error_monitor_errors):
+    config = {
+        "PROPAGATE_EXCEPTIONS": False,
+        "DATABASES": {"default": {"PASSWORD": "123"}},
+    }
+    with app_with_scout(config=config) as app:
+        response = TestApp(app).get("/crash/", expect_errors=True)
+
+    assert response.status_int == 500
+    assert len(error_monitor_errors) == 1
+    error = error_monitor_errors[0]
+    assert error["environment"]["SECRET_KEY"] == "[FILTERED]"
+    assert error["environment"]["DATABASES"]["default"]["PASSWORD"] == "[FILTERED]"