implement transaction_ignore_urls (#923)

* handle transaction_ignore_urls setting [WIP]

* added framework specific tests

(and fixed issues revealed by the tests...)

* implement "outcome" property for transactions and spans

This implements elastic/apm#299.

Additionally, the "status_code" attribute has been added to HTTP spans.

* fix some tests

* change default outcome for spans to "unknown"

* added API functions for setting transaction outcome

* rework outcome API, and make sure it works for unsampled transactions

* fix some tests

* fix a test and add one for testing override behavior

* add an override=False that went forgotten

* expand docs a bit

* implement transaction_ignore_urls [WIP]

* do less work in aiohttp if we're not tracing a transaction

* construct path to json tests in a platform independent way

* fix merge issues

* address review

elasticapm/base.py

@@ -583,6 +583,13 @@ def load_processors(self):
         # setdefault has the nice property that it returns the value that it just set on the dict
         return [seen.setdefault(path, import_string(path)) for path in processors if path not in seen]
 
+    def should_ignore_url(self, url):
+        if self.config.transaction_ignore_urls:
+            for pattern in self.config.transaction_ignore_urls:
+                if pattern.match(url):
+                    return True
+        return False
+
     def check_python_version(self):
         v = tuple(map(int, platform.python_version_tuple()[:2]))
         if v == (2, 7):

elasticapm/conf/__init__.py

@@ -556,6 +556,7 @@ class Config(_ConfigBase):
     instrument_django_middleware = _BoolConfigValue("INSTRUMENT_DJANGO_MIDDLEWARE", default=True)
     autoinsert_django_middleware = _BoolConfigValue("AUTOINSERT_DJANGO_MIDDLEWARE", default=True)
     transactions_ignore_patterns = _ListConfigValue("TRANSACTIONS_IGNORE_PATTERNS", default=[])
+    transaction_ignore_urls = _ListConfigValue("TRANSACTION_IGNORE_URLS", type=starmatch_to_regex, default=[])
     service_version = _ConfigValue("SERVICE_VERSION")
     framework_name = _ConfigValue("FRAMEWORK_NAME")
     framework_version = _ConfigValue("FRAMEWORK_VERSION")

elasticapm/contrib/aiohttp/middleware.py

@@ -48,7 +48,8 @@ def tracing_middleware(app):
 
     async def handle_request(request, handler):
         elasticapm_client = app.get(CLIENT_KEY)
-        if elasticapm_client:
+        should_trace = elasticapm_client and not elasticapm_client.should_ignore_url(request.path)
+        if should_trace:
             request[CLIENT_KEY] = elasticapm_client
             trace_parent = AioHttpTraceParent.from_headers(request.headers)
             elasticapm_client.begin_transaction("request", trace_parent=trace_parent)
@@ -71,11 +72,13 @@ async def handle_request(request, handler):
 
         try:
             response = await handler(request)
-            elasticapm.set_transaction_result("HTTP {}xx".format(response.status // 100), override=False)
-            elasticapm.set_transaction_outcome(http_status_code=response.status, override=False)
-            elasticapm.set_context(
-                lambda: get_data_from_response(response, elasticapm_client.config, constants.TRANSACTION), "response"
-            )
+            if should_trace:
+                elasticapm.set_transaction_result("HTTP {}xx".format(response.status // 100), override=False)
+                elasticapm.set_transaction_outcome(http_status_code=response.status, override=False)
+                elasticapm.set_context(
+                    lambda: get_data_from_response(response, elasticapm_client.config, constants.TRANSACTION),
+                    "response",
+                )
             return response
         except Exception as exc:
             if elasticapm_client:

elasticapm/contrib/django/apps.py

@@ -37,6 +37,7 @@
 from elasticapm.contrib.django.client import get_client
 from elasticapm.utils.disttracing import TraceParent
 from elasticapm.utils.logging import get_logger
+from elasticapm.utils.wsgi import get_current_url
 
 logger = get_logger("elasticapm.traces")
 
@@ -135,17 +136,27 @@ def _request_started_handler(client, sender, *args, **kwargs):
     if not _should_start_transaction(client):
         return
     # try to find trace id
+    trace_parent = None
     if "environ" in kwargs:
+        url = get_current_url(kwargs["environ"], strip_querystring=True, path_only=True)
+        if client.should_ignore_url(url):
+            logger.debug("Ignoring request due to %s matching transaction_ignore_urls")
+            return
         trace_parent = TraceParent.from_headers(
             kwargs["environ"],
             TRACEPARENT_HEADER_NAME_WSGI,
             TRACEPARENT_LEGACY_HEADER_NAME_WSGI,
             TRACESTATE_HEADER_NAME_WSGI,
         )
-    elif "scope" in kwargs and "headers" in kwargs["scope"]:
-        trace_parent = TraceParent.from_headers(kwargs["scope"]["headers"])
-    else:
-        trace_parent = None
+    elif "scope" in kwargs:
+        scope = kwargs["scope"]
+        fake_environ = {"SCRIPT_NAME": scope.get("root_path", ""), "PATH_INFO": scope["path"], "QUERY_STRING": ""}
+        url = get_current_url(fake_environ, strip_querystring=True, path_only=True)
+        if client.should_ignore_url(url):
+            logger.debug("Ignoring request due to %s matching transaction_ignore_urls")
+            return
+        if "headers" in scope:
+            trace_parent = TraceParent.from_headers(scope["headers"])
     client.begin_transaction("request", trace_parent=trace_parent)
 
 

elasticapm/contrib/flask/__init__.py

@@ -178,7 +178,7 @@ def rum_tracing():
             return {}
 
     def request_started(self, app):
-        if not self.app.debug or self.client.config.debug:
+        if (not self.app.debug or self.client.config.debug) and not self.client.should_ignore_url(request.path):
             trace_parent = TraceParent.from_headers(request.headers)
             self.client.begin_transaction("request", trace_parent=trace_parent)
             elasticapm.set_context(

elasticapm/contrib/starlette/__init__.py

@@ -174,11 +174,14 @@ async def _request_started(self, request: Request):
         Args:
             request (Request)
         """
-        trace_parent = TraceParent.from_headers(dict(request.headers))
-        self.client.begin_transaction("request", trace_parent=trace_parent)
+        if not self.client.should_ignore_url(request.url.path):
+            trace_parent = TraceParent.from_headers(dict(request.headers))
+            self.client.begin_transaction("request", trace_parent=trace_parent)
 
-        await set_context(lambda: get_data_from_request(request, self.client.config, constants.TRANSACTION), "request")
-        elasticapm.set_transaction_name("{} {}".format(request.method, request.url.path), override=False)
+            await set_context(
+                lambda: get_data_from_request(request, self.client.config, constants.TRANSACTION), "request"
+            )
+            elasticapm.set_transaction_name("{} {}".format(request.method, request.url.path), override=False)
 
     async def _request_finished(self, response: Response):
         """Captures the end of the request processing to APM.

elasticapm/instrumentation/packages/tornado.py

@@ -52,25 +52,28 @@ async def call(self, module, method, wrapped, instance, args, kwargs):
         from elasticapm.contrib.tornado.utils import get_data_from_request, get_data_from_response
 
         request = instance.request
-        trace_parent = TraceParent.from_headers(request.headers)
         client = instance.application.elasticapm_client
-        client.begin_transaction("request", trace_parent=trace_parent)
-        elasticapm.set_context(
-            lambda: get_data_from_request(instance, request, client.config, constants.TRANSACTION), "request"
-        )
-        # TODO: Can we somehow incorporate the routing rule itself here?
-        elasticapm.set_transaction_name("{} {}".format(request.method, type(instance).__name__), override=False)
+        should_ignore = client.should_ignore_url(request.path)
+        if not should_ignore:
+            trace_parent = TraceParent.from_headers(request.headers)
+            client.begin_transaction("request", trace_parent=trace_parent)
+            elasticapm.set_context(
+                lambda: get_data_from_request(instance, request, client.config, constants.TRANSACTION), "request"
+            )
+            # TODO: Can we somehow incorporate the routing rule itself here?
+            elasticapm.set_transaction_name("{} {}".format(request.method, type(instance).__name__), override=False)
 
         ret = await wrapped(*args, **kwargs)
 
-        elasticapm.set_context(
-            lambda: get_data_from_response(instance, client.config, constants.TRANSACTION), "response"
-        )
-        status = instance.get_status()
-        result = "HTTP {}xx".format(status // 100)
-        elasticapm.set_transaction_result(result, override=False)
-        elasticapm.set_transaction_outcome(http_status_code=status)
-        client.end_transaction()
+        if not should_ignore:
+            elasticapm.set_context(
+                lambda: get_data_from_response(instance, client.config, constants.TRANSACTION), "response"
+            )
+            status = instance.get_status()
+            result = "HTTP {}xx".format(status // 100)
+            elasticapm.set_transaction_result(result, override=False)
+            elasticapm.set_transaction_outcome(http_status_code=status)
+            client.end_transaction()
 
         return ret
 

elasticapm/traces.py

@@ -617,12 +617,6 @@ def begin_transaction(self, transaction_type, trace_parent=None, start=None):
         execution_context.set_transaction(transaction)
         return transaction
 
-    def _should_ignore(self, transaction_name):
-        for pattern in self._ignore_patterns:
-            if pattern.search(transaction_name):
-                return True
-        return False
-
     def end_transaction(self, result=None, transaction_name=None, duration=None):
         """
         End the current transaction and queue it for sending
@@ -643,6 +637,12 @@ def end_transaction(self, result=None, transaction_name=None, duration=None):
             self.queue_func(TRANSACTION, transaction.to_dict())
         return transaction
 
+    def _should_ignore(self, transaction_name):
+        for pattern in self._ignore_patterns:
+            if pattern.search(transaction_name):
+                return True
+        return False
+
 
 class capture_span(object):
     __slots__ = (

elasticapm/utils/__init__.py

@@ -177,6 +177,12 @@ def read_pem_file(file_obj):
 
 
 def starmatch_to_regex(pattern):
+    options = re.DOTALL
+    # check if we are case sensitive
+    if pattern.startswith("(?-i)"):
+        pattern = pattern[5:]
+    else:
+        options |= re.IGNORECASE
     i, n = 0, len(pattern)
     res = []
     while i < n:
@@ -186,4 +192,4 @@ def starmatch_to_regex(pattern):
             res.append(".*")
         else:
             res.append(re.escape(c))
-    return re.compile(r"(?:%s)\Z" % "".join(res), re.IGNORECASE | re.DOTALL)
+    return re.compile(r"(?:%s)\Z" % "".join(res), options)

elasticapm/utils/wsgi.py

@@ -88,7 +88,7 @@ def get_host(environ):
 
 
 # `get_current_url` comes from `werkzeug.wsgi`
-def get_current_url(environ, root_only=False, strip_querystring=False, host_only=False):
+def get_current_url(environ, root_only=False, strip_querystring=False, host_only=False, path_only=False):
     """A handy helper function that recreates the full URL for the current
     request or parts of it.  Here an example:
 
@@ -107,8 +107,12 @@ def get_current_url(environ, root_only=False, strip_querystring=False, host_only
     :param root_only: set `True` if you only want the root URL.
     :param strip_querystring: set to `True` if you don't want the querystring.
     :param host_only: set to `True` if the host URL should be returned.
+    :param path_only: set to `True` if only the path should be returned.
     """
-    tmp = [environ["wsgi.url_scheme"], "://", get_host(environ)]
+    if path_only:
+        tmp = []
+    else:
+        tmp = [environ["wsgi.url_scheme"], "://", get_host(environ)]
     cat = tmp.append
     if host_only:
         return "".join(tmp) + "/"

tests/client/client_tests.py

@@ -373,6 +373,25 @@ def test_ignore_patterns_with_none_transaction_name(elasticapm_client):
     assert t.name == ""
 
 
+@pytest.mark.parametrize(
+    "setting,url,result",
+    [
+        ("", "/foo/bar", False),
+        ("*", "/foo/bar", True),
+        ("/foo/bar,/baz", "/foo/bar", True),
+        ("/foo/bar,/baz", "/baz", True),
+        ("/foo/bar,/bazz", "/baz", False),
+        ("/foo/*/bar,/bazz", "/foo/bar", False),
+        ("/foo/*/bar,/bazz", "/foo/ooo/bar", True),
+        ("*/foo/*/bar,/bazz", "/foo/ooo/bar", True),
+        ("*/foo/*/bar,/bazz", "/baz/foo/ooo/bar", True),
+    ],
+)
+def test_should_ignore_url(elasticapm_client, setting, url, result):
+    elasticapm_client.config.update(1, transaction_ignore_urls=setting)
+    assert elasticapm_client.should_ignore_url(url) is result
+
+
 @pytest.mark.parametrize("sending_elasticapm_client", [{"disable_send": True}], indirect=True)
 def test_disable_send(sending_elasticapm_client):
     assert sending_elasticapm_client.config.disable_send

tests/contrib/asyncio/aiohttp_web_tests.py

@@ -56,6 +56,7 @@ async def boom(request):
 
     app = aiohttp.web.Application()
     app.router.add_route("GET", "/", hello)
+    app.router.add_route("GET", "/hello", hello)
     app.router.add_route("GET", "/boom", boom)
     apm = ElasticAPM(app, elasticapm_client)
     yield apm
@@ -86,6 +87,24 @@ async def test_get(aiohttp_client, aioeapm):
     assert span["name"] == "test"
 
 
+async def test_transaction_ignore_urls(aiohttp_client, aioeapm):
+    app = aioeapm.app
+    client = await aiohttp_client(app)
+    elasticapm_client = aioeapm.client
+    resp = await client.get("/")
+    assert resp.status == 200
+    assert len(elasticapm_client.events[constants.TRANSACTION]) == 1
+    elasticapm_client.config.update(1, transaction_ignore_urls="x")
+    resp = await client.get("/")
+    assert resp.status == 200
+    assert len(elasticapm_client.events[constants.TRANSACTION]) == 2
+    elasticapm_client.config.update(1, transaction_ignore_urls="*,x")
+    resp = await client.get("/")
+    assert resp.status == 200
+    # still only two transaction
+    assert len(elasticapm_client.events[constants.TRANSACTION]) == 2
+
+
 async def test_exception(aiohttp_client, aioeapm):
     app = aioeapm.app
     client = await aiohttp_client(app)
@@ -162,3 +181,15 @@ async def test_traceparent_handling(aiohttp_client, aioeapm):
 async def test_aiohttptraceparent_merge(headers, expected):
     result = AioHttpTraceParent.merge_duplicate_headers(headers, "a")
     assert result == expected
+
+
+async def test_aiohttp_transaction_ignore_urls(aiohttp_client, aioeapm):
+    app = aioeapm.app
+    client = await aiohttp_client(app)
+    elasticapm_client = aioeapm.client
+    resp = await client.get("/hello")
+    assert len(elasticapm_client.events[constants.TRANSACTION]) == 1
+
+    elasticapm_client.config.update(1, transaction_ignore_urls="/*ello,/world")
+    resp = await client.get("/hello")
+    assert len(elasticapm_client.events[constants.TRANSACTION]) == 1

tests/contrib/asyncio/starlette_tests.py

@@ -55,6 +55,12 @@ async def hi(request):
             pass
         return PlainTextResponse("ok")
 
+    @app.route("/hello", methods=["GET", "POST"])
+    async def hello(request):
+        with async_capture_span("test"):
+            pass
+        return PlainTextResponse("ok")
+
     @app.route("/raise-exception", methods=["GET", "POST"])
     async def raise_exception(request):
         raise ValueError()
@@ -210,3 +216,13 @@ def test_capture_headers_body_is_dynamic(app, elasticapm_client):
     assert elasticapm_client.events[constants.TRANSACTION][2]["context"]["request"]["body"] == "[REDACTED]"
     assert "headers" not in elasticapm_client.events[constants.ERROR][1]["context"]["request"]
     assert elasticapm_client.events[constants.ERROR][1]["context"]["request"]["body"] == "[REDACTED]"
+
+
+def test_starlette_transaction_ignore_urls(app, elasticapm_client):
+    client = TestClient(app)
+    response = client.get("/hello")
+    assert len(elasticapm_client.events[constants.TRANSACTION]) == 1
+
+    elasticapm_client.config.update(1, transaction_ignore_urls="/*ello,/world")
+    response = client.get("/hello")
+    assert len(elasticapm_client.events[constants.TRANSACTION]) == 1

tests/contrib/asyncio/tornado/tornado_tests.py

@@ -240,3 +240,15 @@ async def test_no_elasticapm_client(app_no_client, base_url, http_client, elasti
     response = await http_client.fetch(base_url)
     assert response.code == 200
     elasticapm_client.end_transaction("test")
+
+
+@pytest.mark.gen_test
+async def test_tornado_transaction_ignore_urls(app, base_url, http_client):
+    elasticapm_client = app.elasticapm_client
+    response = await http_client.fetch(base_url + "/render")
+    assert len(elasticapm_client.events[constants.TRANSACTION]) == 1
+
+    elasticapm_client.config.update(1, transaction_ignore_urls="/*ender,/bla")
+
+    response = await http_client.fetch(base_url + "/render")
+    assert len(elasticapm_client.events[constants.TRANSACTION]) == 1

tests/contrib/django/django_tests.py

@@ -1591,3 +1591,14 @@ def test_outcome_is_set_for_unsampled_transactions(django_elasticapm_client, cli
         transaction = django_elasticapm_client.events[TRANSACTION][0]
         assert not transaction["sampled"]
         assert transaction["outcome"] == "success"
+
+
+def test_django_ignore_transaction_urls(client, django_elasticapm_client):
+    with override_settings(
+        **middleware_setting(django.VERSION, ["elasticapm.contrib.django.middleware.TracingMiddleware"])
+    ):
+        client.get("/no-error")
+        assert len(django_elasticapm_client.events[TRANSACTION]) == 1
+        django_elasticapm_client.config.update(1, transaction_ignore_urls="/no*")
+        client.get("/no-error")
+    assert len(django_elasticapm_client.events[TRANSACTION]) == 1

tests/contrib/flask/flask_tests.py

@@ -455,3 +455,14 @@ def test_logging_by_level(flask_apm_client):
     assert len(flask_apm_client.client.events[ERROR]) == 1
     error = flask_apm_client.client.events[ERROR][0]
     assert error["log"]["level"] == "error"
+
+
+def test_flask_transaction_ignore_urls(flask_apm_client):
+    resp = flask_apm_client.app.test_client().get("/users/")
+    resp.close()
+    assert len(flask_apm_client.client.events[TRANSACTION]) == 1
+
+    flask_apm_client.client.config.update(version=1, transaction_ignore_urls="/user*")
+    resp = flask_apm_client.app.test_client().get("/users/")
+    resp.close()
+    assert len(flask_apm_client.client.events[TRANSACTION]) == 1