Fix Starlette middleware - providing only static files (#1137)

* Fix Starlette middleware - providing only static files closes #1131 * Fix failing tests * CHANGELOG Co-authored-by: Benjamin Wohlwend <beni@elastic.co> Co-authored-by: Colton Myers <colton.myers@gmail.com>

Author: 3 people

CHANGELOG.asciidoc

@@ -42,6 +42,7 @@ endif::[]
 [float]
 ===== Bug fixes
 
+* Fix for Starlette static files {pull}1137[#1137]
 
 [[release-notes-6.x]]
 === Python Agent version 6.x

elasticapm/contrib/starlette/__init__.py

@@ -237,7 +237,7 @@ def _get_route_name(self, scope, routes, route_name=None):
  if match == Match.FULL:
  route_name = route.path
  child_scope = {**scope, **child_scope}
- if isinstance(route, Mount):
+ if isinstance(route, Mount) and route.routes:
  child_route_name = self._get_route_name(child_scope, route.routes, route_name)
  if child_route_name is None:
  route_name = None

tests/contrib/asyncio/starlette_tests.py

@@ -28,18 +28,18 @@
 # OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
 # OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
 
+import os
 import pytest # isort:skip
 
 from tests.fixtures import TempStoreClient
 
 starlette = pytest.importorskip("starlette") # isort:skip
 
-import types
-
 import mock
 import urllib3
 from starlette.applications import Starlette
 from starlette.responses import PlainTextResponse
+from starlette.staticfiles import StaticFiles
 from starlette.testclient import TestClient
 
 import elasticapm
@@ -53,6 +53,8 @@
 
 starlette_version_tuple = tuple(map(int, starlette.__version__.split(".")[:3]))
 
+file_path, file_name = os.path.split(__file__)
+
 
 @pytest.fixture
 def app(elasticapm_client):
@@ -371,6 +373,77 @@ def test_capture_body_error(app, elasticapm_client):
  response = client.post("/raise-exception", data="[0, 1]")
 
 
+@pytest.fixture
+def app_static_files_only(elasticapm_client):
+ app = Starlette()
+ app.add_middleware(ElasticAPM, client=elasticapm_client)
+ app.mount("/tmp", StaticFiles(directory=file_path), name="static")
+
+ yield app
+
+ elasticapm.uninstrument()
+
+
+def test_static_files_only(app_static_files_only, elasticapm_client):
+ client = TestClient(app_static_files_only)
+
+ response = client.get(
+ "/tmp/" + file_name,
+ headers={
+ constants.TRACEPARENT_HEADER_NAME: "00-0af7651916cd43dd8448eb211c80319c-b7ad6b7169203331-03",
+ constants.TRACESTATE_HEADER_NAME: "foo=bar,bar=baz",
+ "REMOTE_ADDR": "127.0.0.1",
+ },
+ )
+
+ assert response.status_code == 200
+
+ assert len(elasticapm_client.events[constants.TRANSACTION]) == 1
+ transaction = elasticapm_client.events[constants.TRANSACTION][0]
+ spans = elasticapm_client.spans_for_transaction(transaction)
+ assert len(spans) == 0
+
+ assert transaction["name"] == "GET /tmp"
+ assert transaction["result"] == "HTTP 2xx"
+ assert transaction["outcome"] == "success"
+ assert transaction["type"] == "request"
+ assert transaction["span_count"]["started"] == 0
+ assert transaction["context"]["request"]["url"]["pathname"] == "/tmp/" + file_name
+ request = transaction["context"]["request"]
+ assert request["method"] == "GET"
+ assert request["socket"] == {"remote_address": "127.0.0.1", "encrypted": False}
+
+
+def test_static_files_only_file_notfound(app_static_files_only, elasticapm_client):
+ client = TestClient(app_static_files_only)
+
+ response = client.get(
+ "/tmp/whatever",
+ headers={
+ constants.TRACEPARENT_HEADER_NAME: "00-0af7651916cd43dd8448eb211c80319c-b7ad6b7169203331-03",
+ constants.TRACESTATE_HEADER_NAME: "foo=bar,bar=baz",
+ "REMOTE_ADDR": "127.0.0.1",
+ },
+ )
+
+ assert response.status_code == 404
+
+ assert len(elasticapm_client.events[constants.TRANSACTION]) == 1
+ transaction = elasticapm_client.events[constants.TRANSACTION][0]
+ spans = elasticapm_client.spans_for_transaction(transaction)
+ assert len(spans) == 0
+
+ assert transaction["name"] == "GET /tmp"
+ assert transaction["result"] == "HTTP 4xx"
+ assert transaction["outcome"] == "success"
+ assert transaction["type"] == "request"
+ assert transaction["span_count"]["started"] == 0
+ assert transaction["context"]["request"]["url"]["pathname"] == "/tmp/whatever"
+ request = transaction["context"]["request"]
+ assert request["method"] == "GET"
+ assert request["socket"] == {"remote_address": "127.0.0.1", "encrypted": False}
+
+
 def test_make_client_with_config():
  c = make_apm_client(config={"SERVICE_NAME": "foo"}, client_cls=TempStoreClient)
  c.close()

tests/requirements/reqs-starlette-0.13.txt

@@ -1,3 +1,4 @@
 starlette==0.13.0
+aiofiles==0.7.0
 requests==2.23.0
 -r reqs-base.txt

tests/requirements/reqs-starlette-newest.txt

@@ -1,4 +1,5 @@
 starlette
+aiofiles
 requests
 flask
 -r reqs-base.txt