Skip to content
This repository has been archived by the owner on Nov 30, 2022. It is now read-only.

559-endpoint-log-events - Adds middleware for calling analytics events for each endpoint #622

Merged
merged 33 commits into from
Jun 21, 2022
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
33 commits
Select commit Hold shift + click to select a range
2a4dd72
559-endpoint-log-events draft middleware for calling analytics events…
eastandwestwind Jun 6, 2022
aa9dde6
use true source instead of boolean
eastandwestwind Jun 7, 2022
a97f1d0
adds fides source to front end api calls
eastandwestwind Jun 7, 2022
da6e363
remove comment
eastandwestwind Jun 8, 2022
6efda7b
Merge branch 'main' of github.com:ethyca/fidesops into 559-endpoint-l…
eastandwestwind Jun 8, 2022
5bf8937
adds enums
eastandwestwind Jun 9, 2022
f57d6df
adds http middleware, hard-code dev mode in docker-compose
eastandwestwind Jun 13, 2022
3810e85
adds missing source header to login of admin-ui
eastandwestwind Jun 13, 2022
358ad7c
fix conflicts after latest pull
eastandwestwind Jun 14, 2022
70633a9
fix additional front-end admin ui API calls after latest pull
eastandwestwind Jun 14, 2022
4e950c7
pull lates, fix conflicts
eastandwestwind Jun 14, 2022
e385ebd
fix circular imports
eastandwestwind Jun 14, 2022
3d0480d
fix build issues in admin ui
eastandwestwind Jun 14, 2022
d768f13
mypy return type
eastandwestwind Jun 14, 2022
ca8c37e
set dev mode to false in graph test
eastandwestwind Jun 14, 2022
8d2c3e8
pull latest, fix conflicts
eastandwestwind Jun 14, 2022
26ce57e
unintended eslint change
eastandwestwind Jun 14, 2022
1b1af46
adds dev mode false to additional tests
eastandwestwind Jun 15, 2022
4c661c9
turn dev mode on for test_manual_task
eastandwestwind Jun 15, 2022
f3baadf
adds dev mode false to test_execution
eastandwestwind Jun 15, 2022
1cac354
pull latest, fix conflicts
eastandwestwind Jun 15, 2022
8f9d1e2
prettier
eastandwestwind Jun 15, 2022
53f414e
adds unit test for middleware, removes dev mode checks
eastandwestwind Jun 16, 2022
e95ca1d
remove some tests for now, formatting
eastandwestwind Jun 17, 2022
362acd1
pull latest, fix conflicts
eastandwestwind Jun 17, 2022
0397684
bump fideslog version, move main.py test to existing file, add str re…
eastandwestwind Jun 17, 2022
c5e7143
pull latest, fix conflicts
eastandwestwind Jun 17, 2022
0a8f94b
prevents AnalyticsEvent from being called if analytics opt out is true
eastandwestwind Jun 17, 2022
1ec7db7
bump fideslog version, add docstrings, refactor local host method, ad…
eastandwestwind Jun 20, 2022
8adf0e6
format
eastandwestwind Jun 20, 2022
a07880a
isort
eastandwestwind Jun 20, 2022
209c971
unused import
eastandwestwind Jun 20, 2022
8cda76a
hostname is optional type
eastandwestwind Jun 20, 2022
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
38 changes: 10 additions & 28 deletions src/fidesops/main.py
Original file line number Diff line number Diff line change
Expand Up @@ -47,44 +47,26 @@
async def dispatch_log_request(request: Request, call_next: Callable) -> Response:
pattisdr marked this conversation as resolved.
Show resolved Hide resolved
fides_source: Optional[str] = request.headers.get("X-Fides-Source")
pattisdr marked this conversation as resolved.
Show resolved Hide resolved
now: datetime = datetime.now(tz=timezone.utc)
endpoint = f"{request.method}: {request.url.path}"
endpoint = f"{request.method}: {request.url}"

try:
response = await call_next(request)
# HTTPExceptions are considered a handled err by default so are not thrown here.
# Accepted workaround is to inspect status code of response.
# More context- https://github.com/tiangolo/fastapi/issues/1840
try:
if response.status_code >= 400:
response.background = BackgroundTask(
prepare_and_log_request,
endpoint,
response.status_code,
now,
fides_source,
"HTTPException",
)
else:
response.background = BackgroundTask(
prepare_and_log_request,
endpoint,
response.status_code,
now,
fides_source,
None,
)
except Exception as exc:
# always continue if something went wrong with analytics
logger.warning(
f"Analytics event for endpoint {request.url} failed to send: {exc}"
)
return response
response.background = BackgroundTask(
prepare_and_log_request,
endpoint,
response.status_code,
now,
fides_source,
"HTTPException" if response.status_code >= 400 else None
)

return response
except Exception as e:
logger.warning("exception caught")
prepare_and_log_request(
endpoint, e.args[0], now, fides_source, e.__class__.__name__
endpoint, 500, now, fides_source, e.__class__.__name__
)
raise

Expand Down
3 changes: 0 additions & 3 deletions src/fidesops/task/graph_task.py
Original file line number Diff line number Diff line change
Expand Up @@ -63,9 +63,6 @@ def result(*args: Any, **kwargs: Any) -> List[Optional[Row]]:
self = args[0]

raised_ex = None
if config.dev_mode:
# If dev mode, return here so exception isn't caught
return func(*args, **kwargs)
for attempt in range(config.execution.TASK_RETRY_COUNT + 1):
try:
self.skip_if_disabled()
Expand Down
9 changes: 9 additions & 0 deletions tests/conftest.py
Original file line number Diff line number Diff line change
Expand Up @@ -162,3 +162,12 @@ def integration_config() -> MutableMapping[str, Any]:
def celery_enable_logging():
"""Turns on celery output logs."""
return True


@pytest.fixture(autouse=True, scope="session")
def analytics_opt_out():
"""Disable sending analytics when running tests."""
original_value = config.root_user.ANALYTICS_OPT_OUT
config.root_user.ANALYTICS_OPT_OUT = True
yield
config.root_user.ANALYTICS_OPT_OUT = original_value
1 change: 0 additions & 1 deletion tests/graph/test_graph.py
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,6 @@ def test_retry_decorator(privacy_request, policy):
CollectionAddress("postgres_example", "payment_card")
]

config.dev_mode = False
config.execution.TASK_RETRY_COUNT = 5
config.execution.TASK_RETRY_DELAY = 0.1
config.execution.TASK_RETRY_BACKOFF = 0.01
Expand Down
6 changes: 0 additions & 6 deletions tests/integration_tests/test_execution.py
Original file line number Diff line number Diff line change
Expand Up @@ -184,8 +184,6 @@ def test_collection_omitted_on_restart_from_failure(
"""Remove secrets to make privacy request fail, then delete the connection config. Build a graph
that does not contain the deleted dataset config and re-run."""

config.dev_mode = False

integration_mongodb_config.secrets = {}
integration_mongodb_config.save(db)

Expand Down Expand Up @@ -441,8 +439,6 @@ def test_skip_collection_on_restart(
"""Remove secrets to make privacy request fail, then disable connection config
and confirm that datastores are skipped on re-run"""

config.dev_mode = False

integration_mongodb_config.secrets = {}
integration_mongodb_config.save(db)

Expand Down Expand Up @@ -567,8 +563,6 @@ def test_restart_graph_from_failure(
) -> None:
"""Run a failed privacy request and restart from failure"""

config.dev_mode = False

privacy_request = PrivacyRequest(
id=f"test_postgres_access_request_task_{uuid.uuid4()}"
)
Expand Down
1 change: 0 additions & 1 deletion tests/integration_tests/test_manual_task.py
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,6 @@ def test_postgres_with_manual_input_access_request_task(
integration_manual_config,
) -> None:
"""Run a privacy request with two manual nodes"""
config.dev_mode = False
privacy_request = PrivacyRequest(
id=f"test_postgres_access_request_task_{uuid.uuid4()}"
)
Expand Down
2 changes: 0 additions & 2 deletions tests/integration_tests/test_sql_task.py
Original file line number Diff line number Diff line change
Expand Up @@ -956,7 +956,6 @@ def test_retry_access_request(
policy,
integration_postgres_config,
):
config.dev_mode = False
config.execution.TASK_RETRY_COUNT = 1
config.execution.TASK_RETRY_DELAY = 0.1
config.execution.TASK_RETRY_BACKOFF = 0.01
Expand Down Expand Up @@ -1008,7 +1007,6 @@ def test_retry_erasure(
policy,
integration_postgres_config,
):
config.dev_mode = False
config.execution.TASK_RETRY_COUNT = 2
config.execution.TASK_RETRY_DELAY = 0.1
config.execution.TASK_RETRY_BACKOFF = 0.01
Expand Down
139 changes: 139 additions & 0 deletions tests/test_main.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,139 @@
import asyncio
from datetime import datetime, timezone
from typing import Any
from unittest import mock
from unittest.mock import Mock

import pytest
from fideslog.sdk.python.event import AnalyticsEvent
from starlette.requests import Request
from starlette.responses import Response

from fidesops.analytics import in_docker_container, running_on_local_host
from fidesops.main import dispatch_log_request
from fidesops.schemas.analytics import Event
from fidesops.util.async_util import wait_for


class CustomTestException(BaseException):
"""Mock Non-HTTP Exception"""
pass


@mock.patch("fidesops.analytics.send_analytics_event")
@mock.patch("datetime.datetime")
def test_dispatch_log_request_no_exception(
datetime_mock: Mock,
send_analytics_request_mock: Mock,
) -> None:
request = Request(
scope={
"type": "http",
"method": 'GET',
"path": "/testing",
"server": {
"host": "www.testsite.com",
"port": 3000
},
"headers": {}
},
)

datetime_mock.now.return_value = datetime(2022, 5, 22, tzinfo=timezone.utc)

f = asyncio.Future()
f.set_result(Response(status_code=200))

def mock_call_next(request: Request) -> Any:
Copy link
Contributor Author

@eastandwestwind eastandwestwind Jun 17, 2022

Choose a reason for hiding this comment

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

@pattisdr I'm unsure if you'd have any pointers here, but I'm struggling to mock the return of an async function, while also preserving the BackgroundTask that is added to the response here- 53f414e#diff-51868caf44d288309a19f7b59360f148adfa01226f70bf528441cef723439ffcR57

Currently, this code will fail at send_analytics_request_mock.assert_called_once_with() because the background task is not getting added properly using this async mock.

Copy link
Contributor

Choose a reason for hiding this comment

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

Copy link
Contributor Author

Choose a reason for hiding this comment

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

checking this out!

return f

wait_for(dispatch_log_request(request, mock_call_next))
assert send_analytics_request_mock.assert_called_once_with(
AnalyticsEvent(
docker=in_docker_container(),
event=Event.endpoint_call.value,
event_created_at=datetime_mock.now.return_value,
local_host=running_on_local_host(),
endpoint="GET: http://www.testsite.com/testing",
status_code=200,
)
)


@mock.patch("fidesops.analytics.send_analytics_event")
@mock.patch("datetime.datetime")
def test_dispatch_log_request_http_exception(
datetime_mock: Mock,
send_analytics_request_mock: Mock,
) -> None:
request = Request(
scope={
"type": "http",
"method": 'GET',
"path": "/testing",
"server": {
"host": "www.testsite.com",
"port": 3000
},
"headers": {}
},
)

datetime_mock.now.return_value = datetime(2022, 5, 22, tzinfo=timezone.utc)

f = asyncio.Future()
f.set_result(Response(status_code=404))

def mock_call_next(request: Request) -> Any:
return f

wait_for(dispatch_log_request(request, mock_call_next))
assert send_analytics_request_mock.assert_called_once_with(
AnalyticsEvent(
docker=in_docker_container(),
event=Event.endpoint_call.value,
event_created_at=datetime_mock.now.return_value,
local_host=running_on_local_host(),
endpoint="GET: http://www.testsite.com/testing",
status_code=404,
)
)


@mock.patch("fidesops.analytics.send_analytics_event")
@mock.patch("datetime.datetime")
def test_dispatch_log_request_other_exception(
datetime_mock: Mock,
send_analytics_request_mock: Mock,
) -> None:
request = Request(
scope={
"type": "http",
"method": 'GET',
"path": "/testing",
"server": {
"host": "www.testsite.com",
"port": 3000
},
"headers": {}
},
)

datetime_mock.now.return_value = datetime(2022, 5, 22)

def mock_call_next(request: Request) -> Any:
raise CustomTestException(BaseException)

with pytest.raises(CustomTestException):
wait_for(dispatch_log_request(request, mock_call_next))
assert send_analytics_request_mock.assert_called_once_with(
AnalyticsEvent(
docker=in_docker_container(),
event=Event.endpoint_call.value,
event_created_at=datetime_mock.now.return_value,
local_host=running_on_local_host(),
endpoint="GET: http://www.testsite.com/testing",
status_code=500,
error="CustomTestException"
)
)