diff --git a/docs/dev/testing.md b/docs/dev/testing.md index b693b0469..48dcef682 100644 --- a/docs/dev/testing.md +++ b/docs/dev/testing.md @@ -46,6 +46,15 @@ def test_with_suggestion_request(srequest: SuggestionRequestFixture) -> None: result: list[BaseSuggestion] = await provider.query(request) ``` +#### ScopeFixture, ReceiveMockFixture & SendMockFixture +For use when testing middleware, these fixtures initialize or mock the common Scope, +Receive and Send object dependencies. + +_**Usage:**_ +```python +def test_middleware(scope: Scope, receive_mock: Receive, send_mock: Send) -> None: + pass +```` ## Integration Tests @@ -89,6 +98,25 @@ def test_with_test_client_with_event(client_with_events: TestClient): response: Response = client_with_events.get("/api/v1/endpoint") ``` +#### RequestSummaryLogDataFixture +This fixture will extract the extra log data from a captured 'request.summary' +LogRecord for verification + +_**Usage:**_ +```python +def test_with_log_data( + caplog: LogCaptureFixture, + filter_caplog: FilterCaplogFixture, + extract_request_summary_log_data: LogDataFixture +): + records: list[LogRecord] = filter_caplog(caplog.records, "request.summary") + assert len(records) == 1 + + record: LogRecord = records[0] + log_data: dict[str, Any] = extract_request_summary_log_data(record) + assert log_data == expected_log_data +``` + #### InjectProvidersFixture & ProvidersFixture These fixture will setup and teardown given providers. diff --git a/merino/middleware/featureflags.py b/merino/middleware/featureflags.py index 63293be46..96857489b 100644 --- a/merino/middleware/featureflags.py +++ b/merino/middleware/featureflags.py @@ -16,7 +16,7 @@ def __init__(self, app: ASGIApp) -> None: async def __call__(self, scope: Scope, receive: Receive, send: Send) -> None: """Insert session id before handing request""" - if scope["type"] != "http": # pragma: no cover + if scope["type"] != "http": await self.app(scope, receive, send) return diff --git a/merino/middleware/logging.py b/merino/middleware/logging.py index fa61a874a..d5b32d22d 100644 --- a/merino/middleware/logging.py +++ b/merino/middleware/logging.py @@ -5,11 +5,13 @@ from datetime import datetime from typing import Pattern -from starlette.datastructures import Headers from starlette.requests import Request from starlette.types import ASGIApp, Message, Receive, Scope, Send -from merino.middleware import ScopeKey +from merino.util.log_data_creators import ( + create_request_summary_log_data, + create_suggest_log_data, +) # web.suggest.request is used for logs coming from the /suggest endpoint suggest_request_logger = logging.getLogger("web.suggest.request") @@ -24,61 +26,24 @@ class LoggingMiddleware: """An ASGI middleware for logging.""" def __init__(self, app: ASGIApp) -> None: - """Initilize.""" + """Initialize the middleware and store the ASGI app instance.""" self.app = app async def __call__(self, scope: Scope, receive: Receive, send: Send): """Log requests.""" - if scope["type"] != "http": # pragma: no cover + if scope["type"] != "http": await self.app(scope, receive, send) return async def send_wrapper(message: Message) -> None: if message["type"] == "http.response.start": request = Request(scope=scope) + dt: datetime = datetime.fromtimestamp(time.time()) if PATTERN.match(request.url.path): - location = scope[ScopeKey.GEOLOCATION] - ua = scope[ScopeKey.USER_AGENT] - data = { - "sensitive": True, - "path": request.url.path, - "method": request.method, - "query": request.query_params.get("q"), - "errno": 0, - "code": message["status"], - "time": datetime.fromtimestamp(time.time()).isoformat(), - # Provided by the asgi-correlation-id middleware. - "rid": Headers(scope=message)["X-Request-ID"], - "session_id": request.query_params.get("sid"), - "sequence_no": int(seq) - if (seq := request.query_params.get("seq")) - else None, - "country": location.country, - "region": location.region, - "city": location.city, - "dma": location.dma, - "client_variants": request.query_params.get( - "client_variants", "" - ), - "requested_providers": request.query_params.get( - "providers", "" - ), - "browser": ua.browser, - "os_family": ua.os_family, - "form_factor": ua.form_factor, - } + data = create_suggest_log_data(request, message, dt) suggest_request_logger.info("", extra=data) else: - data = { - "agent": request.headers.get("User-Agent"), - "path": request.url.path, - "method": request.method, - "lang": request.headers.get("Accept-Language"), - "querystring": dict(request.query_params), - "errno": 0, - "code": message["status"], - "time": datetime.fromtimestamp(time.time()).isoformat(), - } + data = create_request_summary_log_data(request, message, dt) logger.info("", extra=data) await send(message) diff --git a/merino/middleware/metrics.py b/merino/middleware/metrics.py index cb7042471..5c42f1960 100644 --- a/merino/middleware/metrics.py +++ b/merino/middleware/metrics.py @@ -25,7 +25,7 @@ def __init__(self, app: ASGIApp) -> None: async def __call__(self, scope: Scope, receive: Receive, send: Send) -> None: """Wrap the request with metrics.""" - if scope["type"] != "http": # pragma: no cover + if scope["type"] != "http": await self.app(scope, receive, send) return diff --git a/merino/middleware/user_agent.py b/merino/middleware/user_agent.py index 9bb811800..94e6919db 100644 --- a/merino/middleware/user_agent.py +++ b/merino/middleware/user_agent.py @@ -42,7 +42,7 @@ async def __call__(self, scope: Scope, receive: Receive, send: Send) -> None: """Parse user agent information through "User-Agent" and store the result to `scope`. """ - if scope["type"] != "http": # pragma: no cover + if scope["type"] != "http": await self.app(scope, receive, send) return diff --git a/merino/util/log_data_creators.py b/merino/util/log_data_creators.py new file mode 100644 index 000000000..96fe39cba --- /dev/null +++ b/merino/util/log_data_creators.py @@ -0,0 +1,73 @@ +"""A utility module for lag data creation""" +from datetime import datetime +from typing import Any + +from starlette.datastructures import Headers +from starlette.requests import Request +from starlette.types import Message + +from merino.middleware import ScopeKey +from merino.middleware.geolocation import Location +from merino.middleware.user_agent import UserAgent + + +def create_request_summary_log_data( + request: Request, message: Message, dt: datetime +) -> dict[str, Any]: + """Create log data for API endpoints.""" + general_data = { + "errno": 0, + "time": dt.isoformat(), + } + + request_data = { + "agent": request.headers.get("User-Agent"), + "path": request.url.path, + "method": request.method, + "lang": request.headers.get("Accept-Language"), + "querystring": dict(request.query_params), + "code": message["status"], + } + + return {**general_data, **request_data} + + +def create_suggest_log_data( + request: Request, message: Message, dt: datetime +) -> dict[str, Any]: + """Create log data for the suggest API endpoint.""" + general_data = { + "sensitive": True, + "errno": 0, + "time": dt.isoformat(), + } + + request_data = { + "path": request.url.path, + "method": request.method, + "query": request.query_params.get("q"), + "code": message["status"], + # Provided by the asgi-correlation-id middleware. + "rid": Headers(scope=message)["X-Request-ID"], + "session_id": request.query_params.get("sid"), + "sequence_no": int(seq) if (seq := request.query_params.get("seq")) else None, + "client_variants": request.query_params.get("client_variants", ""), + "requested_providers": request.query_params.get("providers", ""), + } + + location: Location = request.scope[ScopeKey.GEOLOCATION] + location_data = { + "country": location.country, + "region": location.region, + "city": location.city, + "dma": location.dma, + } + + user_agent: UserAgent = request.scope[ScopeKey.USER_AGENT] + user_agent_data = { + "browser": user_agent.browser, + "os_family": user_agent.os_family, + "form_factor": user_agent.form_factor, + } + + return {**general_data, **request_data, **location_data, **user_agent_data} diff --git a/poetry.lock b/poetry.lock index 5cebeb581..cd41cfa0e 100644 --- a/poetry.lock +++ b/poetry.lock @@ -28,7 +28,7 @@ multidict = ">=4.5,<7.0" yarl = ">=1.0,<2.0" [package.extras] -speedups = ["Brotli", "aiodns", "cchardet"] +speedups = ["aiodns", "brotli", "cchardet"] [[package]] name = "aiosignal" @@ -54,8 +54,8 @@ idna = ">=2.8" sniffio = ">=1.1" [package.extras] -doc = ["packaging", "sphinx-autodoc-typehints (>=1.2.0)", "sphinx-rtd-theme"] -test = ["contextlib2", "coverage[toml] (>=4.5)", "hypothesis (>=4.0)", "mock (>=4)", "pytest (>=7.0)", "pytest-mock (>=3.6.1)", "trustme", "uvloop (<0.15)", "uvloop (>=0.15)"] +doc = ["packaging", "sphinx-rtd-theme", "sphinx-autodoc-typehints (>=1.2.0)"] +test = ["coverage[toml] (>=4.5)", "hypothesis (>=4.0)", "pytest (>=7.0)", "pytest-mock (>=3.6.1)", "trustme", "contextlib2", "uvloop (<0.15)", "mock (>=4)", "uvloop (>=0.15)"] trio = ["trio (>=0.16)"] [[package]] @@ -86,10 +86,10 @@ optional = false python-versions = ">=3.5" [package.extras] -dev = ["cloudpickle", "coverage[toml] (>=5.0.2)", "furo", "hypothesis", "mypy (>=0.900,!=0.940)", "pre-commit", "pympler", "pytest (>=4.3.0)", "pytest-mypy-plugins", "sphinx", "sphinx-notfound-page", "zope.interface"] -docs = ["furo", "sphinx", "sphinx-notfound-page", "zope.interface"] -tests = ["cloudpickle", "coverage[toml] (>=5.0.2)", "hypothesis", "mypy (>=0.900,!=0.940)", "pympler", "pytest (>=4.3.0)", "pytest-mypy-plugins", "zope.interface"] -tests-no-zope = ["cloudpickle", "coverage[toml] (>=5.0.2)", "hypothesis", "mypy (>=0.900,!=0.940)", "pympler", "pytest (>=4.3.0)", "pytest-mypy-plugins"] +dev = ["coverage[toml] (>=5.0.2)", "hypothesis", "pympler", "pytest (>=4.3.0)", "mypy (>=0.900,!=0.940)", "pytest-mypy-plugins", "zope.interface", "furo", "sphinx", "sphinx-notfound-page", "pre-commit", "cloudpickle"] +docs = ["furo", "sphinx", "zope.interface", "sphinx-notfound-page"] +tests = ["coverage[toml] (>=5.0.2)", "hypothesis", "pympler", "pytest (>=4.3.0)", "mypy (>=0.900,!=0.940)", "pytest-mypy-plugins", "zope.interface", "cloudpickle"] +tests_no_zope = ["coverage[toml] (>=5.0.2)", "hypothesis", "pympler", "pytest (>=4.3.0)", "mypy (>=0.900,!=0.940)", "pytest-mypy-plugins", "cloudpickle"] [[package]] name = "backoff" @@ -114,9 +114,9 @@ PyYAML = ">=5.3.1" stevedore = ">=1.20.0" [package.extras] -test = ["beautifulsoup4 (>=4.8.0)", "coverage (>=4.5.4)", "fixtures (>=3.0.0)", "flake8 (>=4.0.0)", "pylint (==1.9.4)", "stestr (>=2.5.0)", "testscenarios (>=0.5.0)", "testtools (>=2.3.0)", "toml"] +test = ["coverage (>=4.5.4)", "fixtures (>=3.0.0)", "flake8 (>=4.0.0)", "stestr (>=2.5.0)", "testscenarios (>=0.5.0)", "testtools (>=2.3.0)", "toml", "beautifulsoup4 (>=4.8.0)", "pylint (==1.9.4)"] toml = ["toml"] -yaml = ["PyYAML"] +yaml = ["pyyaml"] [[package]] name = "black" @@ -164,7 +164,7 @@ optional = false python-versions = ">=3.6.0" [package.extras] -unicode-backport = ["unicodedata2"] +unicode_backport = ["unicodedata2"] [[package]] name = "click" @@ -202,7 +202,7 @@ optional = false python-versions = "*" [package.extras] -test = ["flake8 (==3.7.8)", "hypothesis (==3.55.3)"] +test = ["hypothesis (==3.55.3)", "flake8 (==3.7.8)"] [[package]] name = "coverage" @@ -236,7 +236,7 @@ python-versions = ">=3.7,<4" [package.extras] django = ["django"] -flask = ["blinker", "flask"] +flask = ["flask", "blinker"] sanic = ["sanic"] [[package]] @@ -248,11 +248,11 @@ optional = false python-versions = ">=3.7" [package.extras] -all = ["configobj", "hvac", "redis", "ruamel.yaml"] +all = ["redis", "ruamel.yaml", "configobj", "hvac"] configobj = ["configobj"] ini = ["configobj"] redis = ["redis"] -test = ["codecov", "configobj", "django", "flake8", "flake8-debugger", "flake8-print", "flake8-todo", "flask (>=0.12)", "hvac", "pep8-naming", "pytest", "pytest-cov", "pytest-mock", "pytest-xdist", "python-dotenv", "radon", "redis", "toml"] +test = ["pytest", "pytest-cov", "pytest-xdist", "pytest-mock", "flake8", "pep8-naming", "flake8-debugger", "flake8-print", "flake8-todo", "radon", "flask (>=0.12)", "django", "python-dotenv", "toml", "codecov", "redis", "hvac", "configobj"] toml = ["toml"] vault = ["hvac"] yaml = ["ruamel.yaml"] @@ -270,10 +270,10 @@ pydantic = ">=1.6.2,<1.7 || >1.7,<1.7.1 || >1.7.1,<1.7.2 || >1.7.2,<1.7.3 || >1. starlette = "0.19.1" [package.extras] -all = ["email_validator (>=1.1.1,<2.0.0)", "itsdangerous (>=1.1.0,<3.0.0)", "jinja2 (>=2.11.2,<4.0.0)", "orjson (>=3.2.1,<4.0.0)", "python-multipart (>=0.0.5,<0.0.6)", "pyyaml (>=5.3.1,<7.0.0)", "requests (>=2.24.0,<3.0.0)", "ujson (>=4.0.1,!=4.0.2,!=4.1.0,!=4.2.0,!=4.3.0,!=5.0.0,!=5.1.0,<6.0.0)", "uvicorn[standard] (>=0.12.0,<0.18.0)"] -dev = ["autoflake (>=1.4.0,<2.0.0)", "flake8 (>=3.8.3,<4.0.0)", "passlib[bcrypt] (>=1.7.2,<2.0.0)", "pre-commit (>=2.17.0,<3.0.0)", "python-jose[cryptography] (>=3.3.0,<4.0.0)", "uvicorn[standard] (>=0.12.0,<0.18.0)"] -doc = ["mdx-include (>=1.4.1,<2.0.0)", "mkdocs (>=1.1.2,<2.0.0)", "mkdocs-markdownextradata-plugin (>=0.1.7,<0.3.0)", "mkdocs-material (>=8.1.4,<9.0.0)", "pyyaml (>=5.3.1,<7.0.0)", "typer (>=0.4.1,<0.5.0)"] -test = ["anyio[trio] (>=3.2.1,<4.0.0)", "black (==22.3.0)", "databases[sqlite] (>=0.3.2,<0.6.0)", "email_validator (>=1.1.1,<2.0.0)", "flake8 (>=3.8.3,<4.0.0)", "flask (>=1.1.2,<3.0.0)", "httpx (>=0.14.0,<0.19.0)", "isort (>=5.0.6,<6.0.0)", "mypy (==0.910)", "orjson (>=3.2.1,<4.0.0)", "peewee (>=3.13.3,<4.0.0)", "pytest (>=6.2.4,<7.0.0)", "pytest-cov (>=2.12.0,<4.0.0)", "python-multipart (>=0.0.5,<0.0.6)", "requests (>=2.24.0,<3.0.0)", "sqlalchemy (>=1.3.18,<1.5.0)", "types-dataclasses (==0.6.5)", "types-orjson (==3.6.2)", "types-ujson (==4.2.1)", "ujson (>=4.0.1,!=4.0.2,!=4.1.0,!=4.2.0,!=4.3.0,!=5.0.0,!=5.1.0,<6.0.0)"] +all = ["requests (>=2.24.0,<3.0.0)", "jinja2 (>=2.11.2,<4.0.0)", "python-multipart (>=0.0.5,<0.0.6)", "itsdangerous (>=1.1.0,<3.0.0)", "pyyaml (>=5.3.1,<7.0.0)", "ujson (>=4.0.1,!=4.0.2,!=4.1.0,!=4.2.0,!=4.3.0,!=5.0.0,!=5.1.0,<6.0.0)", "orjson (>=3.2.1,<4.0.0)", "email_validator (>=1.1.1,<2.0.0)", "uvicorn[standard] (>=0.12.0,<0.18.0)"] +dev = ["python-jose[cryptography] (>=3.3.0,<4.0.0)", "passlib[bcrypt] (>=1.7.2,<2.0.0)", "autoflake (>=1.4.0,<2.0.0)", "flake8 (>=3.8.3,<4.0.0)", "uvicorn[standard] (>=0.12.0,<0.18.0)", "pre-commit (>=2.17.0,<3.0.0)"] +doc = ["mkdocs (>=1.1.2,<2.0.0)", "mkdocs-material (>=8.1.4,<9.0.0)", "mdx-include (>=1.4.1,<2.0.0)", "mkdocs-markdownextradata-plugin (>=0.1.7,<0.3.0)", "typer (>=0.4.1,<0.5.0)", "pyyaml (>=5.3.1,<7.0.0)"] +test = ["pytest (>=6.2.4,<7.0.0)", "pytest-cov (>=2.12.0,<4.0.0)", "mypy (==0.910)", "flake8 (>=3.8.3,<4.0.0)", "black (==22.3.0)", "isort (>=5.0.6,<6.0.0)", "requests (>=2.24.0,<3.0.0)", "httpx (>=0.14.0,<0.19.0)", "email_validator (>=1.1.1,<2.0.0)", "sqlalchemy (>=1.3.18,<1.5.0)", "peewee (>=3.13.3,<4.0.0)", "databases[sqlite] (>=0.3.2,<0.6.0)", "orjson (>=3.2.1,<4.0.0)", "ujson (>=4.0.1,!=4.0.2,!=4.1.0,!=4.2.0,!=4.3.0,!=5.0.0,!=5.1.0,<6.0.0)", "python-multipart (>=0.0.5,<0.0.6)", "flask (>=1.1.2,<3.0.0)", "anyio[trio] (>=3.2.1,<4.0.0)", "types-ujson (==4.2.1)", "types-orjson (==3.6.2)", "types-dataclasses (==0.6.5)"] [[package]] name = "filelock" @@ -300,6 +300,17 @@ mccabe = ">=0.6.0,<0.7.0" pycodestyle = ">=2.8.0,<2.9.0" pyflakes = ">=2.4.0,<2.5.0" +[[package]] +name = "freezegun" +version = "1.2.2" +description = "Let your Python tests travel through time" +category = "dev" +optional = false +python-versions = ">=3.6" + +[package.dependencies] +python-dateutil = ">=2.7" + [[package]] name = "frozenlist" version = "1.3.1" @@ -396,8 +407,8 @@ rfc3986 = {version = ">=1.3,<2", extras = ["idna2008"]} sniffio = "*" [package.extras] -brotli = ["brotli", "brotlicffi"] -cli = ["click (>=8.0.0,<9.0.0)", "pygments (>=2.0.0,<3.0.0)", "rich (>=10,<13)"] +brotli = ["brotlicffi", "brotli"] +cli = ["click (>=8.0.0,<9.0.0)", "rich (>=10,<13)", "pygments (>=2.0.0,<3.0.0)"] http2 = ["h2 (>=3,<5)"] socks = ["socksio (>=1.0.0,<2.0.0)"] @@ -437,10 +448,10 @@ optional = false python-versions = ">=3.6.1,<4.0" [package.extras] +pipfile_deprecated_finder = ["pipreqs", "requirementslib"] +requirements_deprecated_finder = ["pipreqs", "pip-api"] colors = ["colorama (>=0.4.3,<0.5.0)"] -pipfile-deprecated-finder = ["pipreqs", "requirementslib"] plugins = ["setuptools"] -requirements-deprecated-finder = ["pip-api", "pipreqs"] [[package]] name = "kinto-http" @@ -513,9 +524,6 @@ category = "dev" optional = false python-versions = ">=2.7,!=3.0.*,!=3.1.*,!=3.2.*,!=3.3.*,!=3.4.*,!=3.5.*,!=3.6.*" -[package.dependencies] -setuptools = "*" - [[package]] name = "packaging" version = "21.3" @@ -552,8 +560,8 @@ optional = false python-versions = ">=3.7" [package.extras] -docs = ["furo (>=2021.7.5b38)", "proselint (>=0.10.2)", "sphinx (>=4)", "sphinx-autodoc-typehints (>=1.12)"] -test = ["appdirs (==1.4.4)", "pytest (>=6)", "pytest-cov (>=2.7)", "pytest-mock (>=3.6)"] +docs = ["furo (>=2021.7.5b38)", "proselint (>=0.10.2)", "sphinx-autodoc-typehints (>=1.12)", "sphinx (>=4)"] +test = ["appdirs (==1.4.4)", "pytest-cov (>=2.7)", "pytest-mock (>=3.6)", "pytest (>=6)"] [[package]] name = "pluggy" @@ -564,8 +572,8 @@ optional = false python-versions = ">=3.6" [package.extras] -dev = ["pre-commit", "tox"] -testing = ["pytest", "pytest-benchmark"] +testing = ["pytest-benchmark", "pytest"] +dev = ["tox", "pre-commit"] [[package]] name = "pre-commit" @@ -664,7 +672,7 @@ optional = false python-versions = ">=3.6.8" [package.extras] -diagrams = ["jinja2", "railroad-diagrams"] +diagrams = ["railroad-diagrams", "jinja2"] [[package]] name = "pytest" @@ -698,7 +706,7 @@ python-versions = ">=3.7" pytest = ">=6.1.0" [package.extras] -testing = ["coverage (>=6.2)", "flaky (>=3.5.0)", "hypothesis (>=5.7.1)", "mypy (>=0.931)", "pytest-trio (>=0.7.0)"] +testing = ["coverage (>=6.2)", "hypothesis (>=5.7.1)", "flaky (>=3.5.0)", "mypy (>=0.931)", "pytest-trio (>=0.7.0)"] [[package]] name = "pytest-cov" @@ -713,7 +721,7 @@ coverage = {version = ">=5.2.1", extras = ["toml"]} pytest = ">=4.6" [package.extras] -testing = ["fields", "hunter", "process-tests", "pytest-xdist", "six", "virtualenv"] +testing = ["virtualenv", "pytest-xdist", "six", "process-tests", "hunter", "fields"] [[package]] name = "pytest-mock" @@ -727,7 +735,18 @@ python-versions = ">=3.7" pytest = ">=5.0" [package.extras] -dev = ["pre-commit", "pytest-asyncio", "tox"] +dev = ["pre-commit", "tox", "pytest-asyncio"] + +[[package]] +name = "python-dateutil" +version = "2.8.2" +description = "Extensions to the standard Python datetime module" +category = "dev" +optional = false +python-versions = "!=3.0.*,!=3.1.*,!=3.2.*,>=2.7" + +[package.dependencies] +six = ">=1.5" [[package]] name = "python-dotenv" @@ -764,7 +783,7 @@ urllib3 = ">=1.21.1,<1.27" [package.extras] socks = ["PySocks (>=1.5.6,!=1.5.7)"] -use-chardet-on-py3 = ["chardet (>=3.0.2,<6)"] +use_chardet_on_py3 = ["chardet (>=3.0.2,<6)"] [[package]] name = "rfc3986" @@ -810,7 +829,7 @@ rich = ">=9.2.0" [[package]] name = "sentry-sdk" -version = "1.9.10" +version = "1.11.0" description = "Python client for Sentry (https://sentry.io)" category = "main" optional = false @@ -830,11 +849,12 @@ chalice = ["chalice (>=1.16.0)"] django = ["django (>=1.8)"] falcon = ["falcon (>=1.4)"] fastapi = ["fastapi (>=0.79.0)"] -flask = ["blinker (>=1.1)", "flask (>=0.11)"] +flask = ["flask (>=0.11)", "blinker (>=1.1)"] httpx = ["httpx (>=0.16.0)"] -pure-eval = ["asttokens", "executing", "pure-eval"] +pure_eval = ["pure-eval", "executing", "asttokens"] +pymongo = ["pymongo (>=3.1)"] pyspark = ["pyspark (>=2.4.4)"] -quart = ["blinker (>=1.1)", "quart (>=0.16.1)"] +quart = ["quart (>=0.16.1)", "blinker (>=1.1)"] rq = ["rq (>=0.6)"] sanic = ["sanic (>=0.8)"] sqlalchemy = ["sqlalchemy (>=1.2)"] @@ -842,17 +862,12 @@ starlette = ["starlette (>=0.19.1)"] tornado = ["tornado (>=5)"] [[package]] -name = "setuptools" -version = "65.5.0" -description = "Easily download, build, install, upgrade, and uninstall Python packages" +name = "six" +version = "1.16.0" +description = "Python 2 and 3 compatibility utilities" category = "dev" optional = false -python-versions = ">=3.7" - -[package.extras] -docs = ["furo", "jaraco.packaging (>=9)", "jaraco.tidelift (>=1.4)", "pygments-github-lexers (==0.0.5)", "rst.linker (>=1.9)", "sphinx (>=3.5)", "sphinx-favicon", "sphinx-hoverxref (<2)", "sphinx-inline-tabs", "sphinx-notfound-page (==0.8.3)", "sphinx-reredirects", "sphinxcontrib-towncrier"] -testing = ["build[virtualenv]", "filelock (>=3.4.0)", "flake8 (<5)", "flake8-2020", "ini2toml[lite] (>=0.9)", "jaraco.envs (>=2.2)", "jaraco.path (>=3.2.0)", "mock", "pip (>=19.1)", "pip-run (>=8.8)", "pytest (>=6)", "pytest-black (>=0.3.7)", "pytest-checkdocs (>=2.4)", "pytest-cov", "pytest-enabler (>=1.3)", "pytest-flake8", "pytest-mypy (>=0.9.1)", "pytest-perf", "pytest-xdist", "tomli-w (>=1.0.0)", "virtualenv (>=13.0.0)", "wheel"] -testing-integration = ["build[virtualenv]", "filelock (>=3.4.0)", "jaraco.envs (>=2.2)", "jaraco.path (>=3.2.0)", "pytest", "pytest-enabler", "pytest-xdist", "tomli", "virtualenv (>=13.0.0)", "wheel"] +python-versions = ">=2.7, !=3.0.*, !=3.1.*, !=3.2.*" [[package]] name = "smmap" @@ -1009,8 +1024,8 @@ optional = false python-versions = ">=2.7, !=3.0.*, !=3.1.*, !=3.2.*, !=3.3.*, !=3.4.*, !=3.5.*, <4" [package.extras] -brotli = ["brotli (>=1.0.9)", "brotlicffi (>=0.8.0)", "brotlipy (>=0.6.0)"] -secure = ["certifi", "cryptography (>=1.3.4)", "idna (>=2.0.0)", "ipaddress", "pyOpenSSL (>=0.14)", "urllib3-secure-extra"] +brotli = ["brotlicffi (>=0.8.0)", "brotli (>=1.0.9)", "brotlipy (>=0.6.0)"] +secure = ["pyOpenSSL (>=0.14)", "cryptography (>=1.3.4)", "idna (>=2.0.0)", "certifi", "urllib3-secure-extra", "ipaddress"] socks = ["PySocks (>=1.5.6,!=1.5.7,<2.0)"] [[package]] @@ -1044,9 +1059,9 @@ optional = false python-versions = ">=3.7" [package.extras] -dev = ["Cython (>=0.29.32,<0.30.0)", "Sphinx (>=4.1.2,<4.2.0)", "aiohttp", "flake8 (>=3.9.2,<3.10.0)", "mypy (>=0.800)", "psutil", "pyOpenSSL (>=22.0.0,<22.1.0)", "pycodestyle (>=2.7.0,<2.8.0)", "pytest (>=3.6.0)", "sphinx-rtd-theme (>=0.5.2,<0.6.0)", "sphinxcontrib-asyncio (>=0.3.0,<0.4.0)"] -docs = ["Sphinx (>=4.1.2,<4.2.0)", "sphinx-rtd-theme (>=0.5.2,<0.6.0)", "sphinxcontrib-asyncio (>=0.3.0,<0.4.0)"] -test = ["Cython (>=0.29.32,<0.30.0)", "aiohttp", "flake8 (>=3.9.2,<3.10.0)", "mypy (>=0.800)", "psutil", "pyOpenSSL (>=22.0.0,<22.1.0)", "pycodestyle (>=2.7.0,<2.8.0)"] +dev = ["Cython (>=0.29.32,<0.30.0)", "pytest (>=3.6.0)", "Sphinx (>=4.1.2,<4.2.0)", "sphinxcontrib-asyncio (>=0.3.0,<0.4.0)", "sphinx-rtd-theme (>=0.5.2,<0.6.0)", "flake8 (>=3.9.2,<3.10.0)", "psutil", "pycodestyle (>=2.7.0,<2.8.0)", "pyOpenSSL (>=22.0.0,<22.1.0)", "mypy (>=0.800)", "aiohttp"] +docs = ["Sphinx (>=4.1.2,<4.2.0)", "sphinxcontrib-asyncio (>=0.3.0,<0.4.0)", "sphinx-rtd-theme (>=0.5.2,<0.6.0)"] +test = ["flake8 (>=3.9.2,<3.10.0)", "psutil", "pycodestyle (>=2.7.0,<2.8.0)", "pyOpenSSL (>=22.0.0,<22.1.0)", "mypy (>=0.800)", "Cython (>=0.29.32,<0.30.0)", "aiohttp"] [[package]] name = "virtualenv" @@ -1107,7 +1122,7 @@ multidict = ">=4.0" [metadata] lock-version = "1.1" python-versions = "^3.10" -content-hash = "ca24f128f88cf905f2d8e130cbe2c7e64cbfc3041db7f98cf2a4cc54862052c9" +content-hash = "82c2c624eb689681b1889e5ba8e283e9e4ac1092e969356e8d5ccfaa3bba3660" [metadata.files] aiodogstatsd = [ @@ -1345,6 +1360,10 @@ flake8 = [ {file = "flake8-4.0.1-py2.py3-none-any.whl", hash = "sha256:479b1304f72536a55948cb40a32dce8bb0ffe3501e26eaf292c7e60eb5e0428d"}, {file = "flake8-4.0.1.tar.gz", hash = "sha256:806e034dda44114815e23c16ef92f95c91e4c71100ff52813adf7132a6ad870d"}, ] +freezegun = [ + {file = "freezegun-1.2.2-py3-none-any.whl", hash = "sha256:ea1b963b993cb9ea195adbd893a48d573fda951b0da64f60883d7e988b606c9f"}, + {file = "freezegun-1.2.2.tar.gz", hash = "sha256:cd22d1ba06941384410cd967d8a99d5ae2442f57dfafeff2fda5de8dc5c05446"}, +] frozenlist = [ {file = "frozenlist-1.3.1-cp310-cp310-macosx_10_9_universal2.whl", hash = "sha256:5f271c93f001748fc26ddea409241312a75e13466b06c94798d1a341cf0e6989"}, {file = "frozenlist-1.3.1-cp310-cp310-macosx_10_9_x86_64.whl", hash = "sha256:9c6ef8014b842f01f5d2b55315f1af5cbfde284eb184075c189fd657c2fd8204"}, @@ -1700,6 +1719,10 @@ pytest-mock = [ {file = "pytest-mock-3.8.2.tar.gz", hash = "sha256:77f03f4554392558700295e05aed0b1096a20d4a60a4f3ddcde58b0c31c8fca2"}, {file = "pytest_mock-3.8.2-py3-none-any.whl", hash = "sha256:8a9e226d6c0ef09fcf20c94eb3405c388af438a90f3e39687f84166da82d5948"}, ] +python-dateutil = [ + {file = "python-dateutil-2.8.2.tar.gz", hash = "sha256:0123cacc1627ae19ddf3c27a5de5bd67ee4586fbdd6440d9748f8abb483d3e86"}, + {file = "python_dateutil-2.8.2-py2.py3-none-any.whl", hash = "sha256:961d03dc3453ebbc59dbdea9e4e11c5651520a876d0f4db161e8674aae935da9"}, +] python-dotenv = [ {file = "python-dotenv-0.21.0.tar.gz", hash = "sha256:b77d08274639e3d34145dfa6c7008e66df0f04b7be7a75fd0d5292c191d79045"}, {file = "python_dotenv-0.21.0-py3-none-any.whl", hash = "sha256:1684eb44636dd462b66c3ee016599815514527ad99965de77f43e0944634a7e5"}, @@ -1712,13 +1735,6 @@ pyyaml = [ {file = "PyYAML-6.0-cp310-cp310-manylinux_2_5_x86_64.manylinux1_x86_64.manylinux_2_12_x86_64.manylinux2010_x86_64.whl", hash = "sha256:f84fbc98b019fef2ee9a1cb3ce93e3187a6df0b2538a651bfb890254ba9f90b5"}, {file = "PyYAML-6.0-cp310-cp310-win32.whl", hash = "sha256:2cd5df3de48857ed0544b34e2d40e9fac445930039f3cfe4bcc592a1f836d513"}, {file = "PyYAML-6.0-cp310-cp310-win_amd64.whl", hash = "sha256:daf496c58a8c52083df09b80c860005194014c3698698d1a57cbcfa182142a3a"}, - {file = "PyYAML-6.0-cp311-cp311-macosx_10_9_x86_64.whl", hash = "sha256:d4b0ba9512519522b118090257be113b9468d804b19d63c71dbcf4a48fa32358"}, - {file = "PyYAML-6.0-cp311-cp311-macosx_11_0_arm64.whl", hash = "sha256:81957921f441d50af23654aa6c5e5eaf9b06aba7f0a19c18a538dc7ef291c5a1"}, - {file = "PyYAML-6.0-cp311-cp311-manylinux_2_17_aarch64.manylinux2014_aarch64.whl", hash = "sha256:afa17f5bc4d1b10afd4466fd3a44dc0e245382deca5b3c353d8b757f9e3ecb8d"}, - {file = "PyYAML-6.0-cp311-cp311-manylinux_2_17_s390x.manylinux2014_s390x.whl", hash = "sha256:dbad0e9d368bb989f4515da330b88a057617d16b6a8245084f1b05400f24609f"}, - {file = "PyYAML-6.0-cp311-cp311-manylinux_2_17_x86_64.manylinux2014_x86_64.whl", hash = "sha256:432557aa2c09802be39460360ddffd48156e30721f5e8d917f01d31694216782"}, - {file = "PyYAML-6.0-cp311-cp311-win32.whl", hash = "sha256:bfaef573a63ba8923503d27530362590ff4f576c626d86a9fed95822a8255fd7"}, - {file = "PyYAML-6.0-cp311-cp311-win_amd64.whl", hash = "sha256:01b45c0191e6d66c470b6cf1b9531a771a83c1c4208272ead47a3ae4f2f603bf"}, {file = "PyYAML-6.0-cp36-cp36m-macosx_10_9_x86_64.whl", hash = "sha256:897b80890765f037df3403d22bab41627ca8811ae55e9a722fd0392850ec4d86"}, {file = "PyYAML-6.0-cp36-cp36m-manylinux_2_17_aarch64.manylinux2014_aarch64.whl", hash = "sha256:50602afada6d6cbfad699b0c7bb50d5ccffa7e46a3d738092afddc1f9758427f"}, {file = "PyYAML-6.0-cp36-cp36m-manylinux_2_17_s390x.manylinux2014_s390x.whl", hash = "sha256:48c346915c114f5fdb3ead70312bd042a953a8ce5c7106d5bfb1a5254e47da92"}, @@ -1773,12 +1789,12 @@ scalene = [ {file = "scalene-1.5.13.tar.gz", hash = "sha256:0077d517249cfa6ad0953d85c9656bdaca57b863159bbb78969987d9bdbb539c"}, ] sentry-sdk = [ - {file = "sentry-sdk-1.9.10.tar.gz", hash = "sha256:4fbace9a763285b608c06f01a807b51acb35f6059da6a01236654e08b0ee81ff"}, - {file = "sentry_sdk-1.9.10-py2.py3-none-any.whl", hash = "sha256:2469240f6190aaebcb453033519eae69cfe8cc602065b4667e18ee14fc1e35dc"}, + {file = "sentry-sdk-1.11.0.tar.gz", hash = "sha256:e7b78a1ddf97a5f715a50ab8c3f7a93f78b114c67307785ee828ef67a5d6f117"}, + {file = "sentry_sdk-1.11.0-py2.py3-none-any.whl", hash = "sha256:f467e6c7fac23d4d42bc83eb049c400f756cd2d65ab44f0cc1165d0c7c3d40bc"}, ] -setuptools = [ - {file = "setuptools-65.5.0-py3-none-any.whl", hash = "sha256:f62ea9da9ed6289bfe868cd6845968a2c854d1427f8548d52cae02a42b4f0356"}, - {file = "setuptools-65.5.0.tar.gz", hash = "sha256:512e5536220e38146176efb833d4a62aa726b7bbff82cfbc8ba9eaa3996e0b17"}, +six = [ + {file = "six-1.16.0-py2.py3-none-any.whl", hash = "sha256:8abb2f1d86890a2dfb989f9a77cfcfd3e47c2a354b01111771326f8aa26e0254"}, + {file = "six-1.16.0.tar.gz", hash = "sha256:1e61c37477a1626458e36f7b1d82aa5c9b094fa4802892072e49de9c60c4c926"}, ] smmap = [ {file = "smmap-5.0.0-py3-none-any.whl", hash = "sha256:2aba19d6a040e78d8b09de5c57e96207b09ed71d8e55ce0959eeee6c8e190d94"}, diff --git a/pyproject.toml b/pyproject.toml index 916cbed93..5d1857778 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -84,6 +84,7 @@ types-PyYAML = "^6.0.11" types-requests = "^2.28.10" types-geoip2 = "^3.0.0" scalene = "^1.5.13" +freezegun = "^1.2.2" [build-system] requires = ["poetry-core>=1.0.0"] diff --git a/tests/integration/api/conftest.py b/tests/integration/api/conftest.py index 98213b2a1..844933310 100644 --- a/tests/integration/api/conftest.py +++ b/tests/integration/api/conftest.py @@ -2,12 +2,16 @@ # License, v. 2.0. If a copy of the MPL was not distributed with this # file, You can obtain one at http://mozilla.org/MPL/2.0/. -from typing import Iterator +"""Module for test fixtures for the integration test directory.""" + +from logging import LogRecord +from typing import Any, Iterator import pytest from starlette.testclient import TestClient from merino.main import app +from tests.integration.api.types import RequestSummaryLogDataFixture @pytest.fixture(name="client") @@ -29,3 +33,26 @@ def fixture_test_client_with_events() -> Iterator[TestClient]: """ with TestClient(app) as client: yield client + + +@pytest.fixture(name="extract_request_summary_log_data") +def fixture_extract_request_summary_log_data() -> RequestSummaryLogDataFixture: + """ + Return a function that will extract the extra log data from a captured + "request.summary" log record + """ + + def extract_request_summary_log_data(record: LogRecord) -> dict[str, Any]: + return { + "name": record.name, + "errno": record.__dict__["errno"], + "time": record.__dict__["time"], + "agent": record.__dict__["agent"], + "path": record.__dict__["path"], + "method": record.__dict__["method"], + "lang": record.__dict__["lang"], + "querystring": record.__dict__["querystring"], + "code": record.__dict__["code"], + } + + return extract_request_summary_log_data diff --git a/tests/integration/api/test_error.py b/tests/integration/api/test_error.py index ed51620dc..30f2c0e71 100644 --- a/tests/integration/api/test_error.py +++ b/tests/integration/api/test_error.py @@ -2,11 +2,19 @@ # License, v. 2.0. If a copy of the MPL was not distributed with this # file, You can obtain one at http://mozilla.org/MPL/2.0/. +"""Integration tests for the Merino __error__ API endpoint.""" + import logging +from logging import LogRecord +from typing import Any +import aiodogstatsd from fastapi.testclient import TestClient +from freezegun import freeze_time from pytest import LogCaptureFixture +from pytest_mock import MockerFixture +from tests.integration.api.types import RequestSummaryLogDataFixture from tests.types import FilterCaplogFixture @@ -22,3 +30,88 @@ def test_error( records = filter_caplog(caplog.records, "merino.web.dockerflow") assert len(records) == 1 assert records[0].message == "The __error__ endpoint was called" + + +@freeze_time("1998-03-31") +def test_error_request_log_data( + caplog: LogCaptureFixture, + filter_caplog: FilterCaplogFixture, + extract_request_summary_log_data: RequestSummaryLogDataFixture, + client: TestClient, +) -> None: + """ + Test that the request log for the '__error__' endpoint contains the required + extra data + """ + caplog.set_level(logging.INFO) + + expected_log_data: dict[str, Any] = { + "name": "request.summary", + "agent": ( + "Mozilla/5.0 (Macintosh; Intel Mac OS X 11.2; rv:85.0)" + " Gecko/20100101 Firefox/103.0" + ), + "path": "/__error__", + "method": "GET", + "lang": "en-US", + "querystring": {}, + "errno": 0, + "code": 500, + "time": "1998-03-31T00:00:00", + } + + client.get( + "/__error__", + headers={ + "accept-language": "en-US", + "User-Agent": ( + "Mozilla/5.0 (Macintosh; Intel Mac OS X 11.2; rv:85.0) " + "Gecko/20100101 Firefox/103.0" + ), + }, + ) + + records: list[LogRecord] = filter_caplog(caplog.records, "request.summary") + assert len(records) == 1 + + record: LogRecord = records[0] + log_data: dict[str, Any] = extract_request_summary_log_data(record) + assert log_data == expected_log_data + + +def test_error_metrics(mocker: MockerFixture, client: TestClient) -> None: + """Test that metrics are recorded for the '__error__' endpoint (status code 500)""" + expected_metric_keys: list[str] = [ + "get.__error__.timing", + "get.__error__.status_codes.500", + "response.status_codes.500", + ] + + report = mocker.patch.object(aiodogstatsd.Client, "_report") + + client.get("/__error__") + + # TODO: Remove reliance on internal details of aiodogstatsd + metric_keys: list[str] = [call.args[0] for call in report.call_args_list] + assert metric_keys == expected_metric_keys + + +def test_error_feature_flags(mocker: MockerFixture, client: TestClient) -> None: + """ + Test that feature flags are not added for the '__error__' endpoint (status code 500) + """ + expected_tags_per_metric: dict[str, list[str]] = { + "get.__error__.timing": [], + "get.__error__.status_codes.500": [], + "response.status_codes.500": [], + } + + report = mocker.patch.object(aiodogstatsd.Client, "_report") + + client.get("/__error__") + + # TODO: Remove reliance on internal details of aiodogstatsd + tags_per_metric: dict[str, list[str]] = { + call.args[0]: [*call.args[3].keys()] for call in report.call_args_list + } + assert tags_per_metric == expected_tags_per_metric diff --git a/tests/integration/api/test_heartbeat.py b/tests/integration/api/test_heartbeat.py index 6862ab93d..05665d995 100644 --- a/tests/integration/api/test_heartbeat.py +++ b/tests/integration/api/test_heartbeat.py @@ -2,13 +2,18 @@ # License, v. 2.0. If a copy of the MPL was not distributed with this # file, You can obtain one at http://mozilla.org/MPL/2.0/. +"""Integration tests for the Merino __heartbeat__ and __lbheartbeat__ API endpoints.""" + import logging +from logging import LogRecord +from typing import Any import pytest +from _pytest.logging import LogCaptureFixture from fastapi.testclient import TestClient -from pytest import LogCaptureFixture -from pytest_mock import MockerFixture +from freezegun import freeze_time +from tests.integration.api.types import RequestSummaryLogDataFixture from tests.types import FilterCaplogFixture @@ -20,25 +25,50 @@ def test_heartbeats(client: TestClient, endpoint: str) -> None: assert response.status_code == 200 -def test_non_suggest_request_logs_contain_required_info( - mocker: MockerFixture, +@freeze_time("1998-03-31") +@pytest.mark.parametrize("endpoint", ["__heartbeat__", "__lbheartbeat__"]) +def test_heartbeat_request_log_data( caplog: LogCaptureFixture, filter_caplog: FilterCaplogFixture, + extract_request_summary_log_data: RequestSummaryLogDataFixture, client: TestClient, + endpoint: str, ) -> None: + """ + Test that the request log for the '__heartbeat__' and '__lbheartbeat__' endpoints + contain the required extra data + """ caplog.set_level(logging.INFO) - # Use a valid IP to avoid the geolocation errors/logs - mock_client = mocker.patch("fastapi.Request.client") - mock_client.host = "127.0.0.1" + expected_log_data: dict[str, Any] = { + "name": "request.summary", + "agent": ( + "Mozilla/5.0 (Macintosh; Intel Mac OS X 11.2; rv:85.0)" + " Gecko/20100101 Firefox/103.0" + ), + "path": f"/{endpoint}", + "method": "GET", + "lang": "en-US", + "querystring": {}, + "errno": 0, + "code": 200, + "time": "1998-03-31T00:00:00", + } - client.get("/__heartbeat__") + client.get( + f"/{endpoint}", + headers={ + "accept-language": "en-US", + "User-Agent": ( + "Mozilla/5.0 (Macintosh; Intel Mac OS X 11.2; rv:85.0) " + "Gecko/20100101 Firefox/103.0" + ), + }, + ) - records = filter_caplog(caplog.records, "request.summary") + records: list[LogRecord] = filter_caplog(caplog.records, "request.summary") assert len(records) == 1 - record = records[0] - assert record.name == "request.summary" - assert "country" not in record.__dict__["args"] - assert "session_id" not in record.__dict__["args"] - assert record.__dict__["path"] == "/__heartbeat__" + record: LogRecord = records[0] + log_data: dict[str, Any] = extract_request_summary_log_data(record) + assert log_data == expected_log_data diff --git a/tests/integration/api/test_version.py b/tests/integration/api/test_version.py index a5d35bac6..0454b25ca 100644 --- a/tests/integration/api/test_version.py +++ b/tests/integration/api/test_version.py @@ -2,9 +2,20 @@ # License, v. 2.0. If a copy of the MPL was not distributed with this # file, You can obtain one at http://mozilla.org/MPL/2.0/. +"""Integration tests for the Merino __version__ API endpoint.""" + +import logging +from logging import LogRecord +from typing import Any + from fastapi.testclient import TestClient +from freezegun import freeze_time +from pytest import LogCaptureFixture from pytest_mock import MockerFixture +from tests.integration.api.types import RequestSummaryLogDataFixture +from tests.types import FilterCaplogFixture + def test_version(client: TestClient) -> None: """Test that the version endpoint is supported to conform to dockerflow""" @@ -24,3 +35,50 @@ def test_version_error(mocker: MockerFixture, client: TestClient) -> None: response = client.get("/__version__") assert response.status_code == 500 + + +@freeze_time("1998-03-31") +def test_version_request_log_data( + caplog: LogCaptureFixture, + filter_caplog: FilterCaplogFixture, + extract_request_summary_log_data: RequestSummaryLogDataFixture, + client: TestClient, +) -> None: + """ + Test that the request log for the '__version__' endpoint contains the required + extra data + """ + caplog.set_level(logging.INFO) + + expected_log_data: dict[str, Any] = { + "name": "request.summary", + "agent": ( + "Mozilla/5.0 (Macintosh; Intel Mac OS X 11.2; rv:85.0)" + " Gecko/20100101 Firefox/103.0" + ), + "path": "/__version__", + "method": "GET", + "lang": "en-US", + "querystring": {}, + "errno": 0, + "code": 200, + "time": "1998-03-31T00:00:00", + } + + client.get( + "/__version__", + headers={ + "accept-language": "en-US", + "User-Agent": ( + "Mozilla/5.0 (Macintosh; Intel Mac OS X 11.2; rv:85.0) " + "Gecko/20100101 Firefox/103.0" + ), + }, + ) + + records: list[LogRecord] = filter_caplog(caplog.records, "request.summary") + assert len(records) == 1 + + record: LogRecord = records[0] + log_data: dict[str, Any] = extract_request_summary_log_data(record) + assert log_data == expected_log_data diff --git a/tests/integration/api/types.py b/tests/integration/api/types.py new file mode 100644 index 000000000..963a55f7d --- /dev/null +++ b/tests/integration/api/types.py @@ -0,0 +1,10 @@ +# This Source Code Form is subject to the terms of the Mozilla Public +# License, v. 2.0. If a copy of the MPL was not distributed with this +# file, You can obtain one at http://mozilla.org/MPL/2.0/. + +"""Module for type declarations for the integration test directory.""" + +from logging import LogRecord +from typing import Any, Callable + +RequestSummaryLogDataFixture = Callable[[LogRecord], dict[str, Any]] diff --git a/tests/integration/api/v1/providers/test_providers.py b/tests/integration/api/v1/providers/test_providers.py index 69938b081..623807cdb 100644 --- a/tests/integration/api/v1/providers/test_providers.py +++ b/tests/integration/api/v1/providers/test_providers.py @@ -2,19 +2,29 @@ # License, v. 2.0. If a copy of the MPL was not distributed with this # file, You can obtain one at http://mozilla.org/MPL/2.0/. +"""Integration tests for the Merino v1 providers API endpoint.""" + +import logging +from logging import LogRecord +from typing import Any + import pytest from fastapi.testclient import TestClient +from freezegun import freeze_time +from pytest import LogCaptureFixture from merino.providers import BaseProvider +from tests.integration.api.types import RequestSummaryLogDataFixture from tests.integration.api.v1.fake_providers import ( HiddenProvider, NonsponsoredProvider, SponsoredProvider, ) +from tests.types import FilterCaplogFixture @pytest.mark.parametrize( - "expected_response, providers", + ["expected_response", "providers"], [ ([], {}), ( @@ -68,10 +78,58 @@ def test_providers( providers: dict[str, BaseProvider], ) -> None: """ - Tests that the response to the 'providers' endpoint is as expected when 0-to-many + Test that the response to the 'providers' endpoint is as expected when 0-to-many providers are registered with different availabilities """ response = client.get("/api/v1/providers") assert response.status_code == 200 assert response.json() == expected_response + + +@freeze_time("1998-03-31") +@pytest.mark.parametrize("providers", [{}]) +def test_providers_request_log_data( + caplog: LogCaptureFixture, + filter_caplog: FilterCaplogFixture, + extract_request_summary_log_data: RequestSummaryLogDataFixture, + client: TestClient, +) -> None: + """ + Test that the request log for the 'providers' endpoint contains the required + extra data + """ + caplog.set_level(logging.INFO) + + expected_log_data: dict[str, Any] = { + "name": "request.summary", + "agent": ( + "Mozilla/5.0 (Macintosh; Intel Mac OS X 11.2; rv:85.0)" + " Gecko/20100101 Firefox/103.0" + ), + "path": "/api/v1/providers", + "method": "GET", + "lang": "en-US", + "querystring": {}, + "errno": 0, + "code": 200, + "time": "1998-03-31T00:00:00", + } + + client.get( + "/api/v1/providers", + headers={ + "accept-language": "en-US", + "User-Agent": ( + "Mozilla/5.0 (Macintosh; Intel Mac OS X 11.2; rv:85.0) " + "Gecko/20100101 Firefox/103.0" + ), + }, + ) + + records: list[LogRecord] = filter_caplog(caplog.records, "request.summary") + assert len(records) == 1 + + record: LogRecord = records[0] + log_data: dict[str, Any] = extract_request_summary_log_data(record) + assert log_data == expected_log_data diff --git a/tests/integration/api/v1/suggest/test_metrics.py b/tests/integration/api/v1/suggest/test_metrics.py deleted file mode 100644 index 261320f00..000000000 --- a/tests/integration/api/v1/suggest/test_metrics.py +++ /dev/null @@ -1,132 +0,0 @@ -# This Source Code Form is subject to the terms of the Mozilla Public -# License, v. 2.0. If a copy of the MPL was not distributed with this -# file, You can obtain one at http://mozilla.org/MPL/2.0/. - -import aiodogstatsd -import pytest -from fastapi.testclient import TestClient -from pytest_mock import MockerFixture - -from tests.integration.api.v1.fake_providers import ( - CorruptProvider, - NonsponsoredProvider, - SponsoredProvider, -) - - -@pytest.mark.parametrize( - "providers", - [ - { - "sponsored-provider": SponsoredProvider(enabled_by_default=True), - "nonsponsored-provider": NonsponsoredProvider(enabled_by_default=True), - } - ], -) -@pytest.mark.parametrize( - ["url", "metric_keys"], - [ - ( - "/api/v1/suggest?q=none", - ["get.api.v1.suggest.timing", "get.api.v1.suggest.status_codes.200"], - ), - ( - "/api/v1/nonono", - ["response.status_codes.404"], - ), - ( - "/api/v1/suggest", - ["get.api.v1.suggest.timing", "get.api.v1.suggest.status_codes.400"], - ), - ("/__error__", ["get.__error__.timing", "get.__error__.status_codes.500"]), - ], -) -def test_metrics( - mocker: MockerFixture, - client: TestClient, - url: str, - metric_keys: list[str], -) -> None: - report = mocker.patch.object(aiodogstatsd.Client, "_report") - - client.get(url) - for metric in metric_keys: - report.assert_any_call(metric, mocker.ANY, mocker.ANY, mocker.ANY, mocker.ANY) - - -@pytest.mark.skip(reason="currently no feature flags in use") -@pytest.mark.parametrize( - ["url", "metric_keys", "tags"], - [ - ( - "/api/v1/suggest?q=none", - [ - "get.api.v1.suggest.timing", - "get.api.v1.suggest.status_codes.200", - "providers.adm.query", - "providers.wiki_fruit.query", - "providers.top_picks.query", - "response.status_codes.200", - ], - [ - "feature_flag.test-perc-enabled", - "feature_flag.test-perc-enabled-session", - ], - ), - ("/api/v1/nonono", ["response.status_codes.404"], []), - ( - "/api/v1/suggest", - [ - "get.api.v1.suggest.timing", - "get.api.v1.suggest.status_codes.400", - "response.status_codes.400", - ], - [], - ), - ( - "/__error__", - [ - "get.__error__.timing", - "get.__error__.status_codes.500", - "response.status_codes.500", - ], - [], - ), - ], - ids=["200_with_feature_flags_tags", "404_no_tags", "400_no_tags", "500_no_tags"], -) -def test_feature_flags( - mocker: MockerFixture, client: TestClient, url: str, metric_keys: list, tags: list -): - """Test that feature flags are added for successful requests.""" - report = mocker.patch.object(aiodogstatsd.Client, "_report") - - client.get(url) - - want = {metric_key: tags for metric_key in metric_keys} - - # TODO: This is not great. We're relying on internal details of aiodogstatsd - # here Can we record calls to the metrics client instead? - got = {call.args[0]: [*call.args[3].keys()] for call in report.call_args_list} - - assert got == want - - -@pytest.mark.parametrize("providers", [{"corrupt": CorruptProvider()}]) -def test_metrics_500(mocker: MockerFixture, client: TestClient) -> None: - """Test that 500 status codes are recorded as metrics.""" - error_msg = "test" - metric_keys = [ - "get.api.v1.suggest.timing", - "get.api.v1.suggest.status_codes.500", - ] - - report = mocker.patch.object(aiodogstatsd.Client, "_report") - - with pytest.raises(RuntimeError) as excinfo: - client.get(f"/api/v1/suggest?q={error_msg}") - - for metric in metric_keys: - report.assert_any_call(metric, mocker.ANY, mocker.ANY, mocker.ANY, mocker.ANY) - - assert str(excinfo.value) == error_msg diff --git a/tests/integration/api/v1/suggest/test_suggest.py b/tests/integration/api/v1/suggest/test_suggest.py index 9acb68185..c8f32aa4d 100644 --- a/tests/integration/api/v1/suggest/test_suggest.py +++ b/tests/integration/api/v1/suggest/test_suggest.py @@ -2,14 +2,20 @@ # License, v. 2.0. If a copy of the MPL was not distributed with this # file, You can obtain one at http://mozilla.org/MPL/2.0/. +"""Integration tests for the Merino v1 suggest API endpoint.""" + import logging +from typing import Any +import aiodogstatsd import pytest from fastapi.testclient import TestClient +from freezegun import freeze_time from pytest import LogCaptureFixture from pytest_mock import MockerFixture from tests.integration.api.v1.fake_providers import ( + CorruptProvider, NonsponsoredProvider, SponsoredProvider, ) @@ -19,7 +25,12 @@ @pytest.fixture(name="providers") def fixture_providers() -> Providers: - """Define providers for this module which are injected automatically.""" + """ + Define providers for this module which are injected automatically. + + Note: This fixture will be overridden if a test method has a + 'pytest.mark.parametrize' decorator with a 'providers' definition + """ return { "sponsored-provider": SponsoredProvider(enabled_by_default=True), "nonsponsored-provider": NonsponsoredProvider(enabled_by_default=True), @@ -78,62 +89,100 @@ def test_client_variants(client: TestClient) -> None: assert result["client_variants"] == ["foo", "bar"] -def test_suggest_request_logs_contain_required_info( +@freeze_time("1998-03-31") +def test_suggest_request_log_data( mocker: MockerFixture, caplog: LogCaptureFixture, filter_caplog: FilterCaplogFixture, client: TestClient, ) -> None: - + """ + Tests that the request logs for the 'suggest' endpoint contain the required + extra data + """ caplog.set_level(logging.INFO) # The IP address is taken from `GeoLite2-City-Test.mmdb` mock_client = mocker.patch("fastapi.Request.client") mock_client.host = "216.160.83.56" - query = "nope" - sid = "deadbeef-0000-1111-2222-333344445555" - seq = 0 - client_variants = "foo,bar" - providers = "pro,vider" - root_path = "/api/v1/suggest" + expected_log_data: dict[str, Any] = { + "name": "web.suggest.request", + "sensitive": True, + "errno": 0, + "time": "1998-03-31T00:00:00", + "path": "/api/v1/suggest", + "method": "GET", + "query": "nope", + "code": 200, + "rid": "1b11844c52b34c33a6ad54b7bc2eb7c7", + "session_id": "deadbeef-0000-1111-2222-333344445555", + "sequence_no": 0, + "client_variants": "foo,bar", + "requested_providers": "pro,vider", + "country": "US", + "region": "WA", + "city": "Milton", + "dma": 819, + "browser": "Firefox(103.0)", + "os_family": "macos", + "form_factor": "desktop", + } + client.get( - f"{root_path}?q={query}&sid={sid}&seq={seq}" - f"&client_variants={client_variants}&providers={providers}" + url=expected_log_data["path"], + params={ + "q": expected_log_data["query"], + "sid": expected_log_data["session_id"], + "seq": expected_log_data["sequence_no"], + "client_variants": expected_log_data["client_variants"], + "providers": expected_log_data["requested_providers"], + }, + headers={ + "User-Agent": ( + "Mozilla/5.0 (Macintosh; Intel Mac OS X 11.2; rv:85.0) " + "Gecko/20100101 Firefox/103.0" + ), + "x-request-id": "1b11844c52b34c33a6ad54b7bc2eb7c7", + }, ) records = filter_caplog(caplog.records, "web.suggest.request") - assert len(records) == 1 record = records[0] + log_data: dict[str, Any] = { + "name": record.name, + "sensitive": record.__dict__["sensitive"], + "errno": record.__dict__["errno"], + "time": record.__dict__["time"], + "path": record.__dict__["path"], + "method": record.__dict__["method"], + "query": record.__dict__["query"], + "code": record.__dict__["code"], + "rid": record.__dict__["rid"], + "session_id": record.__dict__["session_id"], + "sequence_no": record.__dict__["sequence_no"], + "client_variants": record.__dict__["client_variants"], + "requested_providers": record.__dict__["requested_providers"], + "country": record.__dict__["country"], + "region": record.__dict__["region"], + "city": record.__dict__["city"], + "dma": record.__dict__["dma"], + "browser": record.__dict__["browser"], + "os_family": record.__dict__["os_family"], + "form_factor": record.__dict__["form_factor"], + } + assert log_data == expected_log_data + - assert record.name == "web.suggest.request" - assert record.__dict__["sensitive"] is True - assert record.__dict__["path"] == root_path - assert record.__dict__["session_id"] == sid - assert record.__dict__["sequence_no"] == seq - assert record.__dict__["query"] == query - assert record.__dict__["client_variants"] == client_variants - assert record.__dict__["requested_providers"] == providers - assert record.__dict__["browser"] == "Other" - assert record.__dict__["os_family"] == "other" - assert record.__dict__["form_factor"] == "other" - assert record.__dict__["country"] == "US" - assert record.__dict__["region"] == "WA" - assert record.__dict__["city"] == "Milton" - assert record.__dict__["dma"] == 819 - - -def test_geolocation_with_invalid_ip( +def test_suggest_with_invalid_geolocation_ip( mocker: MockerFixture, caplog: LogCaptureFixture, filter_caplog: FilterCaplogFixture, client: TestClient, ) -> None: - """ - Test that a warning message is logged if geolocation data is invalid - """ + """Test that a warning message is logged if geolocation data is invalid""" mock_client = mocker.patch("fastapi.Request.client") mock_client.host = "invalid-ip" @@ -143,3 +192,125 @@ def test_geolocation_with_invalid_ip( assert len(records) == 1 assert records[0].message == "Invalid IP address for geolocation parsing" + + +@pytest.mark.parametrize( + ["url", "expected_metric_keys"], + [ + ( + "/api/v1/suggest?q=none", + [ + "providers.sponsored.query", + "providers.non-sponsored.query", + "get.api.v1.suggest.timing", + "get.api.v1.suggest.status_codes.200", + "response.status_codes.200", + ], + ), + ( + "/api/v1/suggest", + [ + "get.api.v1.suggest.timing", + "get.api.v1.suggest.status_codes.400", + "response.status_codes.400", + ], + ), + ], + ids=["status_code_200", "status_code_400"], +) +def test_suggest_metrics( + mocker: MockerFixture, + client: TestClient, + url: str, + expected_metric_keys: list[str], +) -> None: + """ + Test that metrics are recorded for the 'suggest' endpoint (status codes: 200 & 400) + """ + report = mocker.patch.object(aiodogstatsd.Client, "_report") + + client.get(url) + + # TODO: Remove reliance on internal details of aiodogstatsd + metric_keys: list[str] = [call.args[0] for call in report.call_args_list] + assert metric_keys == expected_metric_keys + + +@pytest.mark.parametrize("providers", [{"corrupt": CorruptProvider()}]) +def test_suggest_metrics_500(mocker: MockerFixture, client: TestClient) -> None: + """Test that 500 status codes are recorded as metrics""" + error_msg = "test" + expected_metric_keys = [ + "providers.corrupted.query", + "get.api.v1.suggest.timing", + "get.api.v1.suggest.status_codes.500", + "response.status_codes.500", + ] + + report = mocker.patch.object(aiodogstatsd.Client, "_report") + + with pytest.raises(RuntimeError) as excinfo: + client.get(f"/api/v1/suggest?q={error_msg}") + + # TODO: Remove reliance on internal details of aiodogstatsd + metric_keys: list[str] = [call.args[0] for call in report.call_args_list] + assert metric_keys == expected_metric_keys + + assert str(excinfo.value) == error_msg + + +@pytest.mark.skip(reason="currently no feature flags in use") +@pytest.mark.parametrize( + ["url", "expected_metric_keys", "expected_tags"], + [ + ( + "/api/v1/suggest?q=none", + [ + "get.api.v1.suggest.timing", + "get.api.v1.suggest.status_codes.200", + "providers.adm.query", + "providers.wiki_fruit.query", + "providers.top_picks.query", + "response.status_codes.200", + ], + [ + "feature_flag.test-perc-enabled", + "feature_flag.test-perc-enabled-session", + ], + ), + ( + "/api/v1/suggest", + [ + "get.api.v1.suggest.timing", + "get.api.v1.suggest.status_codes.400", + "response.status_codes.400", + ], + [], + ), + ], + ids=["200_with_feature_flags_tags", "400_no_tags"], +) +def test_suggest_feature_flags( + mocker: MockerFixture, + client: TestClient, + url: str, + expected_metric_keys: list, + expected_tags: list, +): + """ + Test that feature flags are added for the 'suggest' endpoint + (status codes: 200 & 400) + """ + expected_tags_per_metric = { + metric_key: expected_tags for metric_key in expected_metric_keys + } + + report = mocker.patch.object(aiodogstatsd.Client, "_report") + + client.get(url) + + # TODO: Remove reliance on internal details of aiodogstatsd + tags_per_metric = { + call.args[0]: [*call.args[3].keys()] for call in report.call_args_list + } + assert tags_per_metric == expected_tags_per_metric diff --git a/tests/integration/api/v1/suggest/test_top_picks.py b/tests/integration/api/v1/suggest/test_suggest_top_picks.py similarity index 98% rename from tests/integration/api/v1/suggest/test_top_picks.py rename to tests/integration/api/v1/suggest/test_suggest_top_picks.py index 5c9013ca3..4ee1fbc3a 100644 --- a/tests/integration/api/v1/suggest/test_top_picks.py +++ b/tests/integration/api/v1/suggest/test_suggest_top_picks.py @@ -16,7 +16,7 @@ def fixture_providers() -> Providers: @pytest.mark.parametrize( - "query,title,url", + ["query", "title", "url"], [ ("exam", "Example", "https://example.com"), ("exxa", "Example", "https://example.com"), diff --git a/tests/integration/api/v1/suggest/test_wiki_fruit.py b/tests/integration/api/v1/suggest/test_suggest_wiki_fruit.py similarity index 100% rename from tests/integration/api/v1/suggest/test_wiki_fruit.py rename to tests/integration/api/v1/suggest/test_suggest_wiki_fruit.py diff --git a/tests/integration/api/v1/suggest/test_user_agent.py b/tests/integration/api/v1/suggest/test_user_agent.py deleted file mode 100644 index 9b64a07de..000000000 --- a/tests/integration/api/v1/suggest/test_user_agent.py +++ /dev/null @@ -1,78 +0,0 @@ -# This Source Code Form is subject to the terms of the Mozilla Public -# License, v. 2.0. If a copy of the MPL was not distributed with this -# file, You can obtain one at http://mozilla.org/MPL/2.0/. - -import logging - -import pytest -from fastapi.testclient import TestClient -from pytest import LogCaptureFixture -from pytest_mock import MockerFixture - -from tests.integration.api.v1.fake_providers import ( - NonsponsoredProvider, - SponsoredProvider, -) -from tests.integration.api.v1.types import Providers -from tests.types import FilterCaplogFixture - - -@pytest.fixture(name="providers") -def fixture_providers() -> Providers: - """Define providers for this module which are injected automatically.""" - return { - "sponsored-provider": SponsoredProvider(enabled_by_default=True), - "nonsponsored-provider": NonsponsoredProvider(enabled_by_default=True), - } - - -def test_user_agent_middleware( - mocker: MockerFixture, - caplog: LogCaptureFixture, - filter_caplog: FilterCaplogFixture, - client: TestClient, -) -> None: - caplog.set_level(logging.INFO) - - mock_client = mocker.patch("fastapi.Request.client") - mock_client.host = "127.0.0.1" - - headers = { - "User-Agent": ( - "Mozilla/5.0 (Macintosh; Intel Mac OS X 11.2;" - " rv:85.0) Gecko/20100101 Firefox/103.0" - ) - } - client.get("/api/v1/suggest?q=nope", headers=headers) - - records = filter_caplog(caplog.records, "web.suggest.request") - - assert len(records) == 1 - - record = records[0] - assert record.__dict__["browser"] == "Firefox(103.0)" - assert record.__dict__["os_family"] == "macos" - assert record.__dict__["form_factor"] == "desktop" - - -def test_user_agent_middleware_with_missing_ua_str( - mocker: MockerFixture, - caplog: LogCaptureFixture, - filter_caplog: FilterCaplogFixture, - client: TestClient, -) -> None: - caplog.set_level(logging.INFO) - - mock_client = mocker.patch("fastapi.Request.client") - mock_client.host = "127.0.0.1" - - client.get("/api/v1/suggest?q=nope", headers={}) - - records = filter_caplog(caplog.records, "web.suggest.request") - - assert len(records) == 1 - - record = records[0] - assert record.__dict__["browser"] == "Other" - assert record.__dict__["os_family"] == "other" - assert record.__dict__["form_factor"] == "other" diff --git a/tests/integration/api/v1/test_unsupported.py b/tests/integration/api/v1/test_unsupported.py new file mode 100644 index 000000000..8904251f3 --- /dev/null +++ b/tests/integration/api/v1/test_unsupported.py @@ -0,0 +1,100 @@ +# This Source Code Form is subject to the terms of the Mozilla Public +# License, v. 2.0. If a copy of the MPL was not distributed with this +# file, You can obtain one at http://mozilla.org/MPL/2.0/. + +"""Integration tests for unsupported Merino v1 API endpoints.""" + +import logging +from logging import LogRecord +from typing import Any + +import aiodogstatsd +import pytest +from fastapi.testclient import TestClient +from freezegun import freeze_time +from pytest import LogCaptureFixture +from pytest_mock import MockerFixture + +from tests.integration.api.types import RequestSummaryLogDataFixture +from tests.types import FilterCaplogFixture + + +@freeze_time("1998-03-31") +@pytest.mark.parametrize("providers", [{}]) +def test_unsupported_endpoint_request_log_data( + caplog: LogCaptureFixture, + filter_caplog: FilterCaplogFixture, + extract_request_summary_log_data: RequestSummaryLogDataFixture, + client: TestClient, +) -> None: + """ + Test that the request log for unsupported endpoints contains the required extra data + """ + caplog.set_level(logging.INFO) + + expected_log_data: dict[str, Any] = { + "name": "request.summary", + "agent": ( + "Mozilla/5.0 (Macintosh; Intel Mac OS X 11.2; rv:85.0)" + " Gecko/20100101 Firefox/103.0" + ), + "path": "/api/v1/unsupported", + "method": "GET", + "lang": "en-US", + "querystring": {}, + "errno": 0, + "code": 404, + "time": "1998-03-31T00:00:00", + } + + client.get( + "/api/v1/unsupported", + headers={ + "accept-language": "en-US", + "User-Agent": ( + "Mozilla/5.0 (Macintosh; Intel Mac OS X 11.2; rv:85.0) " + "Gecko/20100101 Firefox/103.0" + ), + }, + ) + + records: list[LogRecord] = filter_caplog(caplog.records, "request.summary") + assert len(records) == 1 + + record: LogRecord = records[0] + log_data: dict[str, Any] = extract_request_summary_log_data(record) + assert log_data == expected_log_data + + +@pytest.mark.parametrize("providers", [{}]) +def test_unsupported_endpoint_metrics( + mocker: MockerFixture, client: TestClient +) -> None: + """Test that metrics are recorded for unsupported endpoints (status code 404)""" + expected_metric_keys: list[str] = ["response.status_codes.404"] + + report = mocker.patch.object(aiodogstatsd.Client, "_report") + + client.get("/api/v1/unsupported") + + # TODO: Remove reliance on internal details of aiodogstatsd + metric_keys: list[str] = [call.args[0] for call in report.call_args_list] + assert metric_keys == expected_metric_keys + + +@pytest.mark.parametrize("providers", [{}]) +def test_unsupported_endpoint_flags(mocker: MockerFixture, client: TestClient) -> None: + """ + Test that feature flags are not added for unsupported endpoints (status code 404) + """ + expected_tags_per_metric: dict[str, list[str]] = {"response.status_codes.404": []} + + report = mocker.patch.object(aiodogstatsd.Client, "_report") + + client.get("/api/v1/unsupported") + + # TODO: Remove reliance on internal details of aiodogstatsd + tags_per_metric: dict[str, list[str]] = { + call.args[0]: [*call.args[3].keys()] for call in report.call_args_list + } + assert tags_per_metric == expected_tags_per_metric diff --git a/tests/unit/middleware/conftest.py b/tests/unit/middleware/conftest.py new file mode 100644 index 000000000..10aff3817 --- /dev/null +++ b/tests/unit/middleware/conftest.py @@ -0,0 +1,30 @@ +# This Source Code Form is subject to the terms of the Mozilla Public +# License, v. 2.0. If a copy of the MPL was not distributed with this +# file, You can obtain one at http://mozilla.org/MPL/2.0/. + +"""Module for test fixtures for the middleware unit test directory.""" + +from typing import Any + +import pytest +from pytest_mock import MockerFixture +from starlette.types import Receive, Scope, Send + + +@pytest.fixture(name="scope") +def fixture_scope() -> Scope: + """Create a Scope object for test""" + scope: Scope = {"type": "http"} + return scope + + +@pytest.fixture(name="receive_mock") +def fixture_receive_mock(mocker: MockerFixture) -> Any: + """Create a Receive mock object for test""" + return mocker.AsyncMock(spec=Receive) + + +@pytest.fixture(name="send_mock") +def fixture_send_mock(mocker: MockerFixture) -> Any: + """Create a Send mock object for test""" + return mocker.AsyncMock(spec=Send) diff --git a/tests/unit/middleware/test_featureflags.py b/tests/unit/middleware/test_featureflags.py index 18a2a9b14..03d7713fe 100644 --- a/tests/unit/middleware/test_featureflags.py +++ b/tests/unit/middleware/test_featureflags.py @@ -1,86 +1,63 @@ +# This Source Code Form is subject to the terms of the Mozilla Public +# License, v. 2.0. If a copy of the MPL was not distributed with this +# file, You can obtain one at http://mozilla.org/MPL/2.0/. + +"""Unit tests for the middleware featureflags module.""" + import pytest -from pydantic import ValidationError - -from merino.featureflags import FeatureFlags, session_id_context - - -def test_missing(): - flags = FeatureFlags() - assert not flags.is_enabled("test-missing") - - -def test_enabled(): - flags = FeatureFlags() - assert flags.is_enabled("test-enabled") - - -def test_not_enabled(): - flags = FeatureFlags() - assert not flags.is_enabled("test-not-enabled") - - -def test_no_scheme_no_session_id(caplog): - import logging - - caplog.set_level(logging.ERROR) - flags = FeatureFlags() - assert not flags.is_enabled("test-no-scheme") - - assert len(caplog.records) == 1 - assert caplog.messages[0] == "Expected a session_id but none exist in this context" - - -@pytest.mark.parametrize( - "bucket_id, want", - [ - ("000", True), - ("fff", False), - ], - ids=["session_id_on", "session_id_off"], -) -def test_no_scheme_default_session_id(bucket_id: str, want: bool): - session_id_context.set(bucket_id) - flags = FeatureFlags() - assert flags.is_enabled("test-no-scheme") is want - - -@pytest.mark.parametrize( - "bucket_id, want", - [ - (b"\x00\x00\x00\x00", True), - (b"\xff\xff\xff\xff", False), - ], - ids=["enabed_on", "enabled_off"], -) -def test_enabled_perc(bucket_id: str, want: bool): - """Test for feature flags with "random" scheme and passing in bucket_for value.""" - flags = FeatureFlags() - assert flags.is_enabled("test-perc-enabled", bucket_for=bucket_id) is want - - -@pytest.mark.parametrize( - "bucket_id, want", - [ - ("000", True), - ("fff", False), - ], - ids=["session_id_on", "session_id_off"], -) -def test_enabled_perc_session(bucket_id: str, want: bool): - """Test for feature flags with "random" scheme and passing in bucket_for value.""" - session_id_context.set(bucket_id) - flags = FeatureFlags() - assert flags.is_enabled("test-perc-enabled-session") is want - - -@pytest.mark.parametrize( - "config", - [ - {"invalid-scheme": {"scheme": "invalid", "enabled": 0}}, - {"enabled-range": {"enabled": 42}}, - ], - ids=["invalid-scheme", "enabled-range"], -) -def test_raises_malformed_config(config: dict): - with pytest.raises(ValidationError): - FeatureFlags(flags=config) +from pytest_mock import MockerFixture +from starlette.types import ASGIApp, Receive, Scope, Send + +from merino.featureflags import session_id_context +from merino.middleware.featureflags import FeatureFlagsMiddleware + + +@pytest.fixture(name="featureflags_middleware") +def fixture_featureflags_middleware(mocker: MockerFixture) -> FeatureFlagsMiddleware: + """Creates a FeatureFlagsMiddleware object for test""" + asgiapp_mock = mocker.AsyncMock(spec=ASGIApp) + return FeatureFlagsMiddleware(asgiapp_mock) + + +@pytest.mark.asyncio +async def test_featureflags_sid_found( + featureflags_middleware: FeatureFlagsMiddleware, + scope: Scope, + receive_mock: Receive, + send_mock: Send, +) -> None: + expected_sid: str = "9aadf682-2f7a-4ad1-9976-dc30b60451d8" + scope["query_string"] = f"q=nope&sid={expected_sid}" + + await featureflags_middleware(scope, receive_mock, send_mock) + + assert session_id_context.get() == expected_sid + + +@pytest.mark.asyncio +async def test_featureflags_sid_not_found( + featureflags_middleware: FeatureFlagsMiddleware, + scope: Scope, + receive_mock: Receive, + send_mock: Send, +) -> None: + """Test that SID is assigned `None` is `sid` query parameter is not available.""" + scope["query_string"] = "q=nope" + + await featureflags_middleware(scope, receive_mock, send_mock) + + assert session_id_context.get() is None + + +@pytest.mark.asyncio +async def test_featureflags_invalid_scope_type( + featureflags_middleware: FeatureFlagsMiddleware, + receive_mock: Receive, + send_mock: Send, +) -> None: + """Test that no SID assignment takes place for an unexpected Scope type.""" + scope: Scope = {"type": "not-http"} + + await featureflags_middleware(scope, receive_mock, send_mock) + + assert session_id_context.get() == "fff" diff --git a/tests/unit/middleware/test_geolocation.py b/tests/unit/middleware/test_geolocation.py index 4a23b91a0..a31e277fc 100644 --- a/tests/unit/middleware/test_geolocation.py +++ b/tests/unit/middleware/test_geolocation.py @@ -2,11 +2,11 @@ # License, v. 2.0. If a copy of the MPL was not distributed with this # file, You can obtain one at http://mozilla.org/MPL/2.0/. -from unittest import mock -from unittest.mock import AsyncMock, Mock +"""Unit tests for the middleware geolocation module.""" import pytest from pytest import LogCaptureFixture +from pytest_mock import MockerFixture from starlette.types import ASGIApp, Receive, Scope, Send from merino.middleware import ScopeKey @@ -14,34 +14,15 @@ @pytest.fixture(name="geolocation_middleware") -def fixture_geolocation_middleware() -> GeolocationMiddleware: +def fixture_geolocation_middleware(mocker: MockerFixture) -> GeolocationMiddleware: """Creates a GeolocationMiddleware object for test""" - asgiapp_mock = AsyncMock(spec=ASGIApp) + asgiapp_mock = mocker.AsyncMock(spec=ASGIApp) return GeolocationMiddleware(asgiapp_mock) -@pytest.fixture(name="scope") -def fixture_scope() -> Scope: - """Creates a Scope object for test""" - scope: Scope = {"type": "http"} - return scope - - -@pytest.fixture(name="receive_mock") -def fixture_receive_mock() -> Receive: - """Creates a Receive mock object for test""" - return Mock() - - -@pytest.fixture(name="send_mock") -def fixture_send_mock() -> Send: - """Creates a Send mock object for test""" - return Mock() - - # The first two IP addresses are taken from `GeoLite2-City-Test.mmdb` @pytest.mark.parametrize( - "expected_location, client_ip_and_port", + ["expected_location", "client_ip_and_port"], [ ( Location( @@ -101,6 +82,7 @@ async def test_geolocation_address_not_found( @pytest.mark.asyncio async def test_geolocation_client_ip_override( + mocker: MockerFixture, caplog: LogCaptureFixture, geolocation_middleware: GeolocationMiddleware, scope: Scope, @@ -114,11 +96,9 @@ async def test_geolocation_client_ip_override( expected_location: Location = Location( country="US", region="WA", city="Milton", dma=819, postal_code="98354" ) + mocker.patch("merino.middleware.geolocation.CLIENT_IP_OVERRIDE", "216.160.83.56") - with mock.patch( - "merino.middleware.geolocation.CLIENT_IP_OVERRIDE", "216.160.83.56" - ): - await geolocation_middleware(scope, receive_mock, send_mock) + await geolocation_middleware(scope, receive_mock, send_mock) assert scope[ScopeKey.GEOLOCATION] == expected_location assert len(caplog.messages) == 0 diff --git a/tests/unit/middleware/test_logging.py b/tests/unit/middleware/test_logging.py new file mode 100644 index 000000000..1a1e8f3f1 --- /dev/null +++ b/tests/unit/middleware/test_logging.py @@ -0,0 +1,33 @@ +# This Source Code Form is subject to the terms of the Mozilla Public +# License, v. 2.0. If a copy of the MPL was not distributed with this +# file, You can obtain one at http://mozilla.org/MPL/2.0/. + +"""Unit tests for the middleware logging module.""" + +import logging + +import pytest +from pytest import LogCaptureFixture +from pytest_mock import MockerFixture +from starlette.types import ASGIApp, Receive, Scope, Send + +from merino.middleware.logging import LoggingMiddleware + + +@pytest.mark.asyncio +async def test_logging_invalid_scope_type( + mocker: MockerFixture, + caplog: LogCaptureFixture, + receive_mock: Receive, + send_mock: Send, +) -> None: + """Test that no logging action takes place for an unexpected Scope type.""" + caplog.set_level(logging.INFO) + scope: Scope = {"type": "not-http"} + logging_middleware: LoggingMiddleware = LoggingMiddleware( + mocker.AsyncMock(spec=ASGIApp) + ) + + await logging_middleware(scope, receive_mock, send_mock) + + assert len(caplog.messages) == 0 diff --git a/tests/unit/middleware/test_metrics.py b/tests/unit/middleware/test_metrics.py new file mode 100644 index 000000000..5d0768005 --- /dev/null +++ b/tests/unit/middleware/test_metrics.py @@ -0,0 +1,33 @@ +# This Source Code Form is subject to the terms of the Mozilla Public +# License, v. 2.0. If a copy of the MPL was not distributed with this +# file, You can obtain one at http://mozilla.org/MPL/2.0/. + +"""Unit tests for the middleware metrics module.""" + +import logging + +import pytest +from pytest import LogCaptureFixture +from pytest_mock import MockerFixture +from starlette.types import ASGIApp, Receive, Scope, Send + +from merino.middleware.metrics import MetricsMiddleware + + +@pytest.mark.asyncio +async def test_metrics_invalid_scope_type( + mocker: MockerFixture, + caplog: LogCaptureFixture, + receive_mock: Receive, + send_mock: Send, +) -> None: + """Test that no logging action takes place for an unexpected Scope type.""" + caplog.set_level(logging.INFO) + scope: Scope = {"type": "not-http"} + metrics_middleware: MetricsMiddleware = MetricsMiddleware( + mocker.AsyncMock(spec=ASGIApp) + ) + + await metrics_middleware(scope, receive_mock, send_mock) + + assert len(caplog.messages) == 0 diff --git a/tests/unit/middleware/test_user_agent.py b/tests/unit/middleware/test_user_agent.py new file mode 100644 index 000000000..f05934ec3 --- /dev/null +++ b/tests/unit/middleware/test_user_agent.py @@ -0,0 +1,60 @@ +# This Source Code Form is subject to the terms of the Mozilla Public +# License, v. 2.0. If a copy of the MPL was not distributed with this +# file, You can obtain one at http://mozilla.org/MPL/2.0/. + +"""Unit tests for the middleware user_agent module.""" + +import pytest +from pytest_mock import MockerFixture +from starlette.types import ASGIApp, Receive, Scope, Send + +from merino.middleware import ScopeKey +from merino.middleware.user_agent import UserAgent, UserAgentMiddleware + + +@pytest.fixture(name="user_agent_middleware") +def fixture_user_agent_middleware(mocker: MockerFixture) -> UserAgentMiddleware: + """Creates a UserAgentMiddleware object for test""" + asgiapp_mock = mocker.AsyncMock(spec=ASGIApp) + return UserAgentMiddleware(asgiapp_mock) + + +@pytest.mark.asyncio +async def test_user_agent_parsing( + user_agent_middleware: UserAgentMiddleware, + scope: Scope, + receive_mock: Receive, + send_mock: Send, +): + """Test the proper assignment of UserAgent properties given a request IP address.""" + expected_user_agent = UserAgent( + browser="Firefox(103.0)", os_family="macos", form_factor="desktop" + ) + + scope["headers"] = [ + ( + b"user-agent", + ( + b"Mozilla/5.0 (Macintosh; Intel Mac OS X 11.2; rv:85.0) Gecko/20100101" + b" Firefox/103.0" + ), + ) + ] + + await user_agent_middleware(scope, receive_mock, send_mock) + + assert scope[ScopeKey.USER_AGENT] == expected_user_agent + + +@pytest.mark.asyncio +async def test_user_agent_invalid_scope_type( + user_agent_middleware: UserAgentMiddleware, + receive_mock: Receive, + send_mock: Send, +) -> None: + """Test that no user agent assignment takes place for an unexpected Scope type.""" + scope: Scope = {"type": "not-http"} + + await user_agent_middleware(scope, receive_mock, send_mock) + + assert ScopeKey.USER_AGENT not in scope diff --git a/tests/unit/test_featureflags.py b/tests/unit/test_featureflags.py new file mode 100644 index 000000000..7c270af6c --- /dev/null +++ b/tests/unit/test_featureflags.py @@ -0,0 +1,86 @@ +import pytest +from pydantic import ValidationError + +from merino.featureflags import FeatureFlags, session_id_context + + +def test_missing(): + flags = FeatureFlags() + assert not flags.is_enabled("test-missing") + + +def test_enabled(): + flags = FeatureFlags() + assert flags.is_enabled("test-enabled") + + +def test_not_enabled(): + flags = FeatureFlags() + assert not flags.is_enabled("test-not-enabled") + + +def test_no_scheme_no_session_id(caplog): + import logging + + caplog.set_level(logging.ERROR) + flags = FeatureFlags() + assert not flags.is_enabled("test-no-scheme") + + assert len(caplog.records) == 1 + assert caplog.messages[0] == "Expected a session_id but none exist in this context" + + +@pytest.mark.parametrize( + ["bucket_id", "want"], + [ + ("000", True), + ("fff", False), + ], + ids=["session_id_on", "session_id_off"], +) +def test_no_scheme_default_session_id(bucket_id: str, want: bool): + session_id_context.set(bucket_id) + flags = FeatureFlags() + assert flags.is_enabled("test-no-scheme") is want + + +@pytest.mark.parametrize( + ["bucket_id", "want"], + [ + (b"\x00\x00\x00\x00", True), + (b"\xff\xff\xff\xff", False), + ], + ids=["enabed_on", "enabled_off"], +) +def test_enabled_perc(bucket_id: str, want: bool): + """Test for feature flags with "random" scheme and passing in bucket_for value.""" + flags = FeatureFlags() + assert flags.is_enabled("test-perc-enabled", bucket_for=bucket_id) is want + + +@pytest.mark.parametrize( + ["bucket_id", "want"], + [ + ("000", True), + ("fff", False), + ], + ids=["session_id_on", "session_id_off"], +) +def test_enabled_perc_session(bucket_id: str, want: bool): + """Test for feature flags with "random" scheme and passing in bucket_for value.""" + session_id_context.set(bucket_id) + flags = FeatureFlags() + assert flags.is_enabled("test-perc-enabled-session") is want + + +@pytest.mark.parametrize( + "config", + [ + {"invalid-scheme": {"scheme": "invalid", "enabled": 0}}, + {"enabled-range": {"enabled": 42}}, + ], + ids=["invalid-scheme", "enabled-range"], +) +def test_raises_malformed_config(config: dict): + with pytest.raises(ValidationError): + FeatureFlags(flags=config) diff --git a/tests/unit/util/__init__.py b/tests/unit/util/__init__.py new file mode 100644 index 000000000..e69de29bb diff --git a/tests/unit/util/test_log_data_creator.py b/tests/unit/util/test_log_data_creator.py new file mode 100644 index 000000000..b83ba0143 --- /dev/null +++ b/tests/unit/util/test_log_data_creator.py @@ -0,0 +1,155 @@ +# This Source Code Form is subject to the terms of the Mozilla Public +# License, v. 2.0. If a copy of the MPL was not distributed with this +# file, You can obtain one at http://mozilla.org/MPL/2.0/. + +"""Unit tests for the log data creator utility module.""" + +from datetime import datetime +from typing import Any + +import pytest +from starlette.requests import Request +from starlette.types import Message + +from merino.middleware import ScopeKey +from merino.middleware.geolocation import Location +from merino.middleware.user_agent import UserAgent +from merino.util.log_data_creators import ( + create_request_summary_log_data, + create_suggest_log_data, +) + + +@pytest.mark.parametrize( + ["headers", "expected_agent", "expected_lang"], + [ + ([], None, None), + ([(b"user-agent", b"curl/7.84.0")], "curl/7.84.0", None), + ([(b"accept-language", b"en-US")], None, "en-US"), + ], + ids=["no_headers", "user_agent_header", "accept_language_header"], +) +def test_create_request_summary_log_data( + headers: list[tuple[bytes, bytes]], + expected_agent: str | None, + expected_lang: str | None, +) -> None: + expected_log_data: dict[str, Any] = { + "errno": 0, + "time": "1998-03-31T00:00:00", + "agent": expected_agent, + "path": "/__heartbeat__", + "method": "GET", + "lang": expected_lang, + "querystring": {}, + "code": "200", + } + request: Request = Request( + scope={ + "type": "http", + "headers": headers, + "method": "GET", + "path": "/__heartbeat__", + "query_string": "", + } + ) + message: Message = {"type": "http.response.start", "status": "200"} + dt: datetime = datetime(1998, 3, 31) + + log_data: dict[str, Any] = create_request_summary_log_data(request, message, dt) + + assert log_data == expected_log_data + + +@pytest.mark.parametrize( + [ + "query", + "expected_query", + "expected_sid", + "expected_client_variants", + "expected_providers", + "expected_seq", + ], + [ + (b"", None, None, "", "", None), + ( + b"q=nope&sid=9aadf682-2f7a-4ad1-9976-dc30b60451d8", + "nope", + "9aadf682-2f7a-4ad1-9976-dc30b60451d8", + "", + "", + None, + ), + (b"q=nope&client_variants=foo,bar", "nope", None, "foo,bar", "", None), + (b"q=nope&providers=testprovider", "nope", None, "", "testprovider", None), + (b"q=nope&seq=0", "nope", None, "", "", 0), + ], + ids=[ + "no_query", + "query_with_sid", + "query_with_client_variants", + "query_with_providers", + "query_with_seq", + ], +) +def test_create_suggest_log_data( + query: bytes, + expected_query: str, + expected_sid: str | None, + expected_client_variants: str, + expected_providers: str, + expected_seq: int | None, +) -> None: + location: Location = Location( + country="US", region="WA", city="Milton", dma=819, postal_code="98354" + ) + user_agent: UserAgent = UserAgent( + browser="Firefox(103.0)", os_family="macos", form_factor="desktop" + ) + expected_log_data: dict[str, Any] = { + "sensitive": True, + "errno": 0, + "time": "1998-03-31T00:00:00", + "path": "/api/v1/suggest", + "method": "GET", + "query": expected_query, + "code": "200", + "rid": "1b11844c52b34c33a6ad54b7bc2eb7c7", + "session_id": expected_sid, + "sequence_no": expected_seq, + "client_variants": expected_client_variants, + "requested_providers": expected_providers, + "country": location.country, + "region": location.region, + "city": location.city, + "dma": location.dma, + "browser": user_agent.browser, + "os_family": user_agent.os_family, + "form_factor": user_agent.form_factor, + } + request: Request = Request( + scope={ + "type": "http", + "headers": [], + "method": "GET", + "path": "/api/v1/suggest", + "query_string": query, + ScopeKey.GEOLOCATION: location, + ScopeKey.USER_AGENT: user_agent, + } + ) + message: Message = { + "type": "http.response.start", + "status": "200", + "headers": [ + (b"content-length", b"119"), + (b"content-type", b"application/json"), + (b"x-request-id", b"1b11844c52b34c33a6ad54b7bc2eb7c7"), + (b"access-control-expose-headers", b"X-Request-ID"), + ], + } + dt: datetime = datetime(1998, 3, 31) + + log_data: dict[str, Any] = create_suggest_log_data(request, message, dt) + + assert log_data == expected_log_data diff --git a/tests/unit/test_user_agent.py b/tests/unit/util/test_user_agent_parsing.py similarity index 96% rename from tests/unit/test_user_agent.py rename to tests/unit/util/test_user_agent_parsing.py index 5d65155a8..c58a2d99c 100644 --- a/tests/unit/test_user_agent.py +++ b/tests/unit/util/test_user_agent_parsing.py @@ -77,7 +77,7 @@ @pytest.mark.parametrize( - "ua,expected_browser,expected_os_family,expected_form_factor", + ["ua", "expected_browser", "expected_os_family", "expected_form_factor"], SCENARIOS, ) def test_ua_parsing(ua, expected_browser, expected_os_family, expected_form_factor):