Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(python): add ASGI support (Django) #1099

Open
wants to merge 5 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion .github/workflows/python.yml
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,8 @@ jobs:
node-version: 20

- run: npm ci --ignore-scripts
- run: make test-metrics-python-django
- run: make test-metrics-python-django-wsgi
- run: make test-metrics-python-django-asgi
- run: make test-metrics-python-flask
- run: make test-webhooks-python-flask

Expand Down
9 changes: 7 additions & 2 deletions Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -66,8 +66,13 @@ test-webhooks-php-laravel: ## Run webhooks tests against the PHP SDK + Laravel
## Python
##

test-metrics-python-django: ## Run Metrics tests against the Python SDK + Django
docker compose up --build --detach integration_metrics_python_django
test-metrics-python-django-wsgi: ## Run Metrics tests against the Python SDK + Django
docker compose up --build --detach integration_metrics_python_django_wsgi
SUPPORTS_HASHING=true npm run test:integration-metrics || make cleanup-failure
@make cleanup

test-metrics-python-django-asgi: ## Run Metrics tests against the Python SDK + Django
docker compose up --build --detach integration_metrics_python_django_asgi
SUPPORTS_HASHING=true npm run test:integration-metrics || make cleanup-failure
@make cleanup

Expand Down
14 changes: 12 additions & 2 deletions docker-compose.yml
Original file line number Diff line number Diff line change
Expand Up @@ -86,10 +86,20 @@ services:
#
# Python
#
integration_metrics_python_django:
integration_metrics_python_django_wsgi:
build:
context: .
dockerfile: ./test/integrations/python/django.Dockerfile
dockerfile: ./test/integrations/python/django-wsgi.Dockerfile
ports:
- 8000:8000
extra_hosts: *default-extra_hosts
environment:
<<: *server-config

integration_metrics_python_django_asgi:
build:
context: .
dockerfile: ./test/integrations/python/django-asgi.Dockerfile
ports:
- 8000:8000
extra_hosts: *default-extra_hosts
Expand Down
7 changes: 5 additions & 2 deletions packages/python/Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -12,8 +12,11 @@ lint: ## Run code standard checks
lint-fix: ## Run code formatting checks
black .

serve-metrics-django: ## Start the local Django server to test Metrics
README_API_KEY=$(API_KEY) python3 examples/metrics_django/manage.py runserver
serve-metrics-django-wsgi: ## Start the local Django WSGI server to test Metrics
README_API_KEY=$(API_KEY) SERVER_TYPE="wsgi" python3 examples/metrics_django/manage.py runserver

serve-metrics-django-asgi: ## Start the local Django ASGI server to test Metrics
README_API_KEY=$(API_KEY) SERVER_TYPE="asgi" python3 examples/metrics_django/manage.py runserver

serve-metrics-flask: ## Start the local Flask server to test Metrics
README_API_KEY=$(API_KEY) python3 examples/flask/app.py
Expand Down
44 changes: 31 additions & 13 deletions packages/python/examples/metrics_django/manage.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,6 @@
import os
import sys

from django.core.management.commands.runserver import Command as runserver

if os.getenv("README_API_KEY") is None and "runserver" in sys.argv:
sys.stderr.write("Missing `README_API_KEY` environment variable")
sys.stderr.flush()
Expand All @@ -14,18 +12,38 @@
def main():
"""Run administrative tasks."""
os.environ.setdefault("DJANGO_SETTINGS_MODULE", "metrics_django.settings")
runserver.default_addr = "0.0.0.0"
runserver.default_port = os.getenv("PORT") or 8000
try:
host = "0.0.0.0"
port = os.getenv("PORT") or 8000

server_type = os.getenv("SERVER_TYPE", "wsgi").lower()
llimllib marked this conversation as resolved.
Show resolved Hide resolved
if server_type == "wsgi":
# pylint: disable=import-outside-toplevel
from django.core.management import execute_from_command_line
except ImportError as exc:
raise ImportError(
"Couldn't import Django. Are you sure it's installed and "
"available on your PYTHONPATH environment variable? Did you "
"forget to activate a virtual environment?"
) from exc
execute_from_command_line(sys.argv)
from django.core.management.commands.runserver import Command as runserver

runserver.default_addr = host
runserver.default_port = port
try:
# pylint: disable=import-outside-toplevel
from django.core.management import execute_from_command_line
except ImportError as exc:
raise ImportError(
"Couldn't import Django. Are you sure it's installed and "
"available on your PYTHONPATH environment variable? Did you "
"forget to activate a virtual environment?"
) from exc
execute_from_command_line(sys.argv)
else:
try:
# pylint: disable=import-outside-toplevel
import uvicorn
except ImportError as exc:
raise ImportError(
"Couldn't import Uvicorn. Are you sure it's installed and "
"available on your PYTHONPATH environment variable?"
) from exc
uvicorn.run(
"metrics_django.asgi:application", host=host, port=port, lifespan="off"
)


if __name__ == "__main__":
Expand Down
11 changes: 8 additions & 3 deletions packages/python/readme_metrics/Metrics.py
Original file line number Diff line number Diff line change
Expand Up @@ -59,13 +59,18 @@ def process(self, request, response: ResponseInfoWrapper) -> None:
"""Enqueues a request/response combination to be submitted the API.

Args:
request (Request): Request object from your WSGI server
request (Request): Request object from your WSGI/ASGI server
response (ResponseInfoWrapper): Response object
"""
if not self.host_allowed(request.environ["HTTP_HOST"]):
if hasattr(request, "environ"):
http_host = request.environ["HTTP_HOST"]
else:
http_host = request.headers.get("host")

if not self.host_allowed(http_host):
# pylint: disable=C0301
self.config.LOGGER.debug(
f"Not enqueueing request, host {request.environ['HTTP_HOST']} not in ALLOWED_HTTP_HOSTS"
f"Not enqueueing request, host {http_host} not in ALLOWED_HTTP_HOSTS"
)
return

Expand Down
40 changes: 33 additions & 7 deletions packages/python/readme_metrics/PayloadBuilder.py
Original file line number Diff line number Diff line change
Expand Up @@ -75,10 +75,17 @@ def __call__(
if group is None:
return None

if hasattr(request, "environ"):
client_ip_address = request.environ.get("REMOTE_ADDR")
else:
client_ip_address = (
request.scope["client"][0] if hasattr(request, "scope") else None
)

payload = {
"_id": logId,
"group": group,
"clientIPAddress": request.environ.get("REMOTE_ADDR"),
"clientIPAddress": client_ip_address,
"development": self.development_mode,
"request": {
"log": {
Expand Down Expand Up @@ -164,7 +171,8 @@ def _build_request_payload(self, request) -> dict:

Args:
request (Request): Request object containing the request information, either
a `werkzeug.Request` or a `django.core.handlers.wsgi.WSGIRequest`.
a `werkzeug.Request`, `django.core.handlers.wsgi.WSGIRequest`, or
a `django.core.handlers.asgi.ASGIRequest`.

Returns:
dict: Wrapped request payload
Expand Down Expand Up @@ -214,10 +222,20 @@ def _build_request_payload(self, request) -> dict:
if "Authorization" in headers:
headers["Authorization"] = mask(headers["Authorization"])

if hasattr(request, "environ"):
http_version = request.environ["SERVER_PROTOCOL"]
elif hasattr(request, "scope"):
http_version = f'HTTP/{request.scope["http_version"]}'
else:
http_version = "HTTP/1.1" # Assume it is default HTTP version
llimllib marked this conversation as resolved.
Show resolved Hide resolved
self.logger.warning(
"Unable to detect the HTTP version. Setting default to 'HTTP/1.1'."
)

payload = {
"method": request.method,
"url": self._build_base_url(request),
"httpVersion": request.environ["SERVER_PROTOCOL"],
"httpVersion": http_version,
"headers": [{"name": k, "value": v} for (k, v) in headers.items()],
"headersSize": -1,
"queryString": [{"name": k, "value": v} for (k, v) in queryString],
Expand Down Expand Up @@ -276,9 +294,11 @@ def _get_query_string(self, request):
if hasattr(request, "query_string"):
# works for Werkzeug request objects only
result = request.query_string
elif "QUERY_STRING" in request.environ:
elif hasattr(request, "environ") and "QUERY_STRING" in request.environ:
# works for Django, and possibly other request objects too
result = request.environ["QUERY_STRING"]
elif hasattr(request, "scope"):
result = request.scope["query_string"]
else:
raise QueryNotFound(
"Don't know how to retrieve query string from this type of request"
Expand Down Expand Up @@ -311,18 +331,24 @@ def _build_base_url(self, request):

scheme, host, path = None, None, None

if "wsgi.url_scheme" in request.environ:
if hasattr(request, "environ") and "wsgi.url_scheme" in request.environ:
scheme = request.environ["wsgi.url_scheme"]
elif hasattr(request, "scheme"):
scheme = request.scheme

# pylint: disable=protected-access
if hasattr(request, "_get_raw_host"):
# Django request objects already have a properly formatted host field
host = request._get_raw_host()
elif "HTTP_HOST" in request.environ:
elif hasattr(request, "environ") and "HTTP_HOST" in request.environ:
host = request.environ["HTTP_HOST"]
elif hasattr(request, "scope"):
host = request.headers.get("host")

if "PATH_INFO" in request.environ:
if hasattr(request, "environ") and "PATH_INFO" in request.environ:
path = request.environ["PATH_INFO"]
elif hasattr(request, "scope"):
path = request.scope["path"]

if scheme and path and host:
if len(query_string) > 0:
Expand Down
29 changes: 25 additions & 4 deletions packages/python/readme_metrics/django.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,23 +2,31 @@
import time

from django.conf import settings
from asgiref.sync import iscoroutinefunction, markcoroutinefunction

from readme_metrics.Metrics import Metrics
from readme_metrics.ResponseInfoWrapper import ResponseInfoWrapper
from readme_metrics import MetricsApiConfig


class MetricsMiddleware:
async_capable = True
sync_capable = True

def __init__(self, get_response, config=None):
self.get_response = get_response
self.config = config or settings.README_METRICS_CONFIG
assert isinstance(self.config, MetricsApiConfig)
self.metrics_core = Metrics(self.config)
if iscoroutinefunction(self.get_response):
markcoroutinefunction(self)

def __call__(self, request):
if request.method == "OPTIONS":
return self.get_response(request)
if iscoroutinefunction(self.get_response):
return self.async_process(request)
return self.sync_process(request)

def preamble(self, request):
try:
request.rm_start_dt = datetime.utcnow().strftime("%Y-%m-%dT%H:%M:%SZ")
request.rm_start_ts = int(time.time() * 1000)
Expand All @@ -30,8 +38,7 @@ def __call__(self, request):
# throw an error. Log it but don't re-raise.
self.config.LOGGER.exception(e)

response = self.get_response(request)

def process_response(self, request, response):
try:
try:
body = response.content.decode("utf-8")
Expand All @@ -50,4 +57,18 @@ def __call__(self, request):
# throw an error. Log it but don't re-raise.
self.config.LOGGER.exception(e)

def sync_process(self, request):
if request.method == "OPTIONS":
return self.get_response(request)
self.preamble(request)
response = self.get_response(request)
self.process_response(request, response)
return response

async def async_process(self, request):
if request.method == "OPTIONS":
return await self.get_response(request)
self.preamble(request)
response = await self.get_response(request)
self.process_response(request, response)
return response
44 changes: 34 additions & 10 deletions packages/python/readme_metrics/tests/django_test.py
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
from datetime import datetime, timedelta
import time
from unittest.mock import Mock
from unittest.mock import Mock, AsyncMock

from readme_metrics import MetricsApiConfig
from readme_metrics.django import MetricsMiddleware
Expand All @@ -15,20 +15,26 @@


class TestDjangoMiddleware:
def test(self):
response = Mock()
def setup_middleware(self, is_async=False):
response = AsyncMock() if is_async else Mock()
response.headers = {"X-Header": "X Value!"}
get_response = Mock(return_value=response)
get_response = (
AsyncMock(return_value=response)
if is_async
else Mock(return_value=response)
)

middleware = MetricsMiddleware(get_response, config=mock_config)
assert middleware.get_response == get_response
middleware.metrics_core = Mock()
return middleware

request = Mock()
request.headers = {"Content-Length": "123"}
middleware(request)
# the middleware should call get_response(request)
get_response.assert_called_once_with(request)
def validate_metrics(self, middleware, request, is_async=False):
if is_async:
# the middleware should await get_response(request)
middleware.get_response.assert_awaited_once_with(request)
else:
# the middleware should call get_response(request)
middleware.get_response.assert_called_once_with(request)

# the middleware should set request.rm_start_dt to roughly the current
# datetime
Expand Down Expand Up @@ -57,6 +63,24 @@ def test(self):
getattr(request, "rm_content_length") == request.headers["Content-Length"]
)

def test_sync(self):
middleware = self.setup_middleware()

request = Mock()
request.headers = {"Content-Length": "123"}
middleware(request)

self.validate_metrics(middleware, request)

async def test_async(self):
middleware = self.setup_middleware(is_async=True)

request = AsyncMock()
request.headers = {"Content-Length": "123"}
await middleware(request)

self.validate_metrics(middleware, request, is_async=True)

def test_missing_content_length(self):
middleware = MetricsMiddleware(Mock(), config=mock_config)
request = Mock()
Expand Down
3 changes: 2 additions & 1 deletion packages/python/requirements.txt
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
Django==4.2.16
Flask==3.0.3
requests==2.32.3
Werkzeug==3.0.4
Werkzeug==3.0.4
uvicorn==0.32.0

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I looked a bit into uvicorn and it seems like it's commonly used (like with FastAPI), but just want to check that it's necessary since it does add another dependency so it should be vetted.

15 changes: 15 additions & 0 deletions test/integrations/python/django-asgi.Dockerfile
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
FROM python:3.10

COPY packages/python /src

# Set up the Python SDK
WORKDIR /src
RUN pip3 install --no-cache-dir -r requirements.txt

# Install Django
WORKDIR /src/examples/metrics_django
RUN pip3 install --no-cache-dir -r requirements.txt

ENV SERVER_TYPE="asgi"

CMD ["python3", "manage.py", "runserver"]