Repository: scoutapp/scout_apm_python
Pull request #467 (2020-02-05 16:42:09)

Remove unnecessary Hug import
Left over from #460.


Patch:
diff --git a/CHANGELOG.md b/CHANGELOG.md
index 066c00e..f5b4b27 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -15,6 +15,11 @@
 
 - Fix Jinja2 asynchronous rendering instrumentation for Jinja2 2.11.0+
   ([PR #462](https://github.com/scoutapp/scout_apm_python/pull/462)).
+- Stop patching `elasticsearch`'s `scripts_painless_context()`, because it has
+  been removed in version 7.5.1
+  ([Issue #454](https://github.com/scoutapp/scout_apm_python/issues/454)).
+- Fix for `elasticsearch` version 7.5.1's change of positional argument order
+  ([Issue #456](https://github.com/scoutapp/scout_apm_python/issues/456)).
 
 ## [2.10.0] 2020-01-02
 
diff --git a/DEVELOPMENT.md b/DEVELOPMENT.md
index 3f2972e..f6bb96e 100644
--- a/DEVELOPMENT.md
+++ b/DEVELOPMENT.md
@@ -58,11 +58,14 @@ Some of the tests require external services to run against. These are specified
 by environment variables. These are:
 
 * `ELASTICSEARCH_URL` - point to a running Elasticsearch instance, e.g.
-  "http://localhost:9200/"
+  "http://localhost:9200/" . You can start it with:
+  `docker run --detach --name elasticsearch --publish 9200:9200 -e "discovery.type=single-node" elasticsearch:7.5.2` .
 * `MONGODB_URL` - point to a running MongoDB instance e.g.
-  "mongodb://localhost:27017/"
+  "mongodb://localhost:27017/" . You can start it with:
+  `docker run --detach --name mongo --publish 27017:27017 mongo:4.0` .
 * `REDIS_URL` - point to a running Redis instance e.g.
-  "redis://localhost:6379/0"
+  "redis://localhost:6379/0" . You can start it with:
+  `docker run --detach --name redis --publish 6379:6379 redis:5` .
 
 You can `export` any of these environment variables and run the respective
 tests with `tox`.
diff --git a/src/scout_apm/compat.py b/src/scout_apm/compat.py
index 9df13fb..050cf6a 100644
--- a/src/scout_apm/compat.py
+++ b/src/scout_apm/compat.py
@@ -63,8 +63,21 @@ else:
     from urlparse import parse_qsl
 
 
+if sys.version_info >= (3, 0):
+
+    def get_pos_args(func):
+        return inspect.getfullargspec(func).args
+
+
+else:
+
+    def get_pos_args(func):
+        return inspect.getargspec(func).args
+
+
 def kwargs_only(func):
     """
+    Source: https://pypi.org/project/kwargs-only/
     Make a function only accept keyword arguments.
     This can be dropped in Python 3 in lieu of:
         def foo(*, bar=default):
diff --git a/src/scout_apm/falcon.py b/src/scout_apm/falcon.py
index a1b09a6..ed0214b 100644
--- a/src/scout_apm/falcon.py
+++ b/src/scout_apm/falcon.py
@@ -14,11 +14,6 @@ from scout_apm.core.web_requests import (
     track_request_queue_time,
 )
 
-try:
-    from hug.interface import HTTP as HugHTTP
-except ImportError:
-    HugHTTP = None
-
 logger = logging.getLogger(__name__)
 
 # Falcon Middleware docs:
diff --git a/src/scout_apm/instruments/elasticsearch.py b/src/scout_apm/instruments/elasticsearch.py
index 08af7bf..8bfdbf4 100644
--- a/src/scout_apm/instruments/elasticsearch.py
+++ b/src/scout_apm/instruments/elasticsearch.py
@@ -6,6 +6,7 @@ from collections import namedtuple
 
 import wrapt
 
+from scout_apm.compat import get_pos_args
 from scout_apm.core.tracked_request import TrackedRequest
 
 try:
@@ -57,7 +58,6 @@ CLIENT_METHODS = [
     ClientMethod("reindex", False),
     ClientMethod("reindex_rethrottle", False),
     ClientMethod("render_search_template", False),
-    ClientMethod("scripts_painless_context", False),
     ClientMethod("scripts_painless_execute", False),
     ClientMethod("scroll", False),
     ClientMethod("search", True),
@@ -86,34 +86,50 @@ def ensure_client_instrumented():
                     wrapped = wrap_client_method(method)
                 setattr(Elasticsearch, name, wrapped)
             except Exception as exc:
-                # Workaround for version 7.5.0 removing scripts_painless_context:
-                # https://github.com/elastic/elasticsearch-py/issues/1098
-                if name != "scripts_painless_context":
-                    logger.warning(
-                        "Unable to instrument elasticsearch.Elasticsearch.%s: %r",
-                        name,
-                        exc,
-                        exc_info=exc,
-                    )
+                logger.warning(
+                    "Unable to instrument elasticsearch.Elasticsearch.%s: %r",
+                    name,
+                    exc,
+                    exc_info=exc,
+                )
 
         have_patched_client = True
 
 
 @wrapt.decorator
 def wrap_client_index_method(wrapped, instance, args, kwargs):
-    def _get_index(index, *args, **kwargs):
-        return index
+    # elasticsearch-py 7.5.1 changed the order of arguments for client methods,
+    # so to be safe we need to inspect the wrapped method's positional
+    # arguments to see if we should pull it from there
+    if "index" in kwargs:
+        index = kwargs["index"]
+    else:
+        unwrapped = wrapped
+        while True:
+            try:
+                unwrapped = unwrapped.__wrapped__
+            except AttributeError:
+                break
 
-    try:
-        index = _get_index(*args, **kwargs)
-    except TypeError:
+        pos_args = get_pos_args(unwrapped)
+        try:
+            index_index = pos_args.index("index")
+        except ValueError:  # pragma: no cover
+            # This guards against the method not accepting an 'index' argument
+            # but they all do - for now
+            index = ""
+        else:
+            try:
+                index = args[index_index - 1]  # subtract 'self'
+            except IndexError:
+                index = ""
+
+    if isinstance(index, (list, tuple)):
+        index = ",".join(index)
+    if index == "":
         index = "Unknown"
-    else:
-        if not index:
-            index = "Unknown"
-        if isinstance(index, (list, tuple)):
-            index = ",".join(index)
     index = index.title()
+
     camel_name = "".join(c.title() for c in wrapped.__name__.split("_"))
     operation = "Elasticsearch/{}/{}".format(index, camel_name)
     tracked_request = TrackedRequest.instance()


Test patch:
diff --git a/tests/integration/instruments/test_elasticsearch.py b/tests/integration/instruments/test_elasticsearch.py
index 5537a9c..6e2aa04 100644
--- a/tests/integration/instruments/test_elasticsearch.py
+++ b/tests/integration/instruments/test_elasticsearch.py
@@ -36,10 +36,6 @@ def test_all_client_attributes_accounted_for():
 
 @pytest.mark.parametrize(["method_name"], [[m.name] for m in CLIENT_METHODS])
 def test_all_client_methods_exist(method_name):
-    # Workaround for version 7.5.0 removing scripts_painless_context:
-    # https://github.com/elastic/elasticsearch-py/issues/1098
-    if method_name == "scripts_painless_context":
-        return
     assert hasattr(elasticsearch.Elasticsearch, method_name)
 
 
@@ -49,11 +45,6 @@ def test_all_client_methods_exist(method_name):
     [[m.name, m.takes_index_argument] for m in CLIENT_METHODS],
 )
 def test_all_client_methods_match_index_argument(method_name, takes_index_argument):
-    # Workaround for version 7.5.0 removing scripts_painless_context:
-    # https://github.com/elastic/elasticsearch-py/issues/1098
-    if method_name == "scripts_painless_context":
-        return
-
     signature = inspect.signature(getattr(elasticsearch.Elasticsearch, method_name))
 
     if takes_index_argument:
@@ -158,15 +149,11 @@ def test_search(elasticsearch_client, tracked_request):
     assert span.operation == "Elasticsearch/Unknown/Search"
 
 
-@pytest.mark.xfail(
-    reason=(
-        "Broken argument order in Elasticsearch 7.5.1 - see "
-        "https://github.com/scoutapp/scout_apm_python/issues/456"
-    )
-)
+@skip_if_python_2  # Cannot unwrap decorators on Python 2
 def test_search_arg_named_index(elasticsearch_client, tracked_request):
     with pytest.raises(elasticsearch.exceptions.NotFoundError):
-        elasticsearch_client.search("myindex")
+        # body, index
+        elasticsearch_client.search(None, "myindex")
 
     assert len(tracked_request.complete_spans) == 1
     span = tracked_request.complete_spans[0]
@@ -183,7 +170,8 @@ def test_search_kwarg_named_index(elasticsearch_client, tracked_request):
 
 
 def test_search_arg_empty_index(elasticsearch_client, tracked_request):
-    elasticsearch_client.search("")
+    # body, index
+    elasticsearch_client.search(None, "")
 
     assert len(tracked_request.complete_spans) == 1
     span = tracked_request.complete_spans[0]
diff --git a/tests/unit/test_compat.py b/tests/unit/test_compat.py
index 9294db1..79d9db3 100644
--- a/tests/unit/test_compat.py
+++ b/tests/unit/test_compat.py
@@ -5,7 +5,7 @@ import datetime as dt
 
 import pytest
 
-from scout_apm.compat import ContextDecorator, datetime_to_timestamp, text
+from scout_apm.compat import ContextDecorator, datetime_to_timestamp, get_pos_args, text
 
 
 def test_context_decorator():
@@ -48,3 +48,17 @@ def test_datetime_to_timestamp(given, expected):
 )
 def test_text(given, expected):
     assert text(given) == expected
+
+
+def test_get_pos_args_args():
+    def foo(bar, baz):
+        pass
+
+    assert get_pos_args(foo) == ["bar", "baz"]
+
+
+def test_get_pos_args_kwargs():
+    def foo(bar, baz=None):
+        pass
+
+    assert get_pos_args(foo) == ["bar", "baz"]