Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

AnalyticsEvent.endpoint includes the HTTP request method #66

Merged
merged 14 commits into from
Jun 16, 2022
Merged
Show file tree
Hide file tree
Changes from 9 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -127,3 +127,9 @@ dmypy.json

# Pyre type checker
.pyre/

# PyCharm, IntelliJ etc
.idea/

# Mac
.DS_Store
PSalant726 marked this conversation as resolved.
Show resolved Hide resolved
2 changes: 1 addition & 1 deletion Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ check-all: build-local isort black pylint mypy xenon pytest
@echo "Running formatter, linter, typechecker and tests..."

black:
@$(RUN_NO_DEPS) black --check fideslog/
@$(RUN_NO_DEPS) black --check --exclude="sdk/python/_version\.py" fideslog/

isort:
@echo "Running isort checks..."
Expand Down
2 changes: 1 addition & 1 deletion docker-compose.yml
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ services:
environment:
FIDESLOG__LOGGING_LEVEL: "debug"
FIDESLOG__SERVER_HOST: "fideslog"
FIDESLOG__SERVER_HOT_RELOAD: true
FIDESLOG__SERVER_HOT_RELOAD: "true"
PSalant726 marked this conversation as resolved.
Show resolved Hide resolved
expose:
- 8080
ports:
Expand Down
33 changes: 29 additions & 4 deletions fideslog/api/models/analytics_event.py
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,18 @@

from .manifest_file_counts import ManifestFileCounts

ALLOWED_HTTP_METHODS = [
"CONNECT",
"DELETE",
"GET",
"HEAD",
"OPTIONS",
"PATCH",
"POST",
"PUT",
"TRACE",
]


class AnalyticsEvent(BaseModel):
"""The model for analytics events."""
Expand All @@ -30,7 +42,7 @@ class AnalyticsEvent(BaseModel):
)
endpoint: Optional[str] = Field(
None,
description="For events submitted as a result of making API server requests, the API endpoint path that was requested.",
description="For events submitted as a result of making API server requests, the HTTP method and API endpoint path included on the request, delimited by a colon. Ex: `GET: /api/path`.",
)
error: Optional[str] = Field(
None,
Expand Down Expand Up @@ -81,12 +93,25 @@ def check_not_an_email_address(cls, value: str) -> str:
return value

@validator("endpoint")
def check_no_hostname(cls, value: str) -> str:
def validate_endpoint_format(cls, value: str) -> str:
"""
Ensure that endpoints contain only the URL path.
Ensure that endpoint contains method and path, and that path component contains only the URL path.
"""

return urlparse(value).path
endpoint_components = value.split(":")
assert (
len(endpoint_components) == 2
), "endpoint must contain only the HTTP method and URL path, delimited by a colon"
endpoint_components[0] = endpoint_components[0].strip().upper()
assert (
endpoint_components[0].strip() in ALLOWED_HTTP_METHODS
eastandwestwind marked this conversation as resolved.
Show resolved Hide resolved
), f"HTTP method must be one of {', '.join(ALLOWED_HTTP_METHODS)}"
assert (
endpoint_components[1].strip()
== urlparse(endpoint_components[1].strip()).path
), "endpoint must contain only the URL path"
value_with_uppercase_method = ":".join(endpoint_components)
return value_with_uppercase_method
eastandwestwind marked this conversation as resolved.
Show resolved Hide resolved

@validator("event_created_at")
def check_in_the_past(cls, value: datetime) -> datetime:
Expand Down
Empty file added fideslog/sdk/__init__.py
Empty file.
2 changes: 1 addition & 1 deletion fideslog/sdk/python/__init__.py
Original file line number Diff line number Diff line change
@@ -1,3 +1,3 @@
from . import _version

__version__ = _version.get_versions()["version"]
__version__ = _version.get_versions()["version"] # type: ignore
PSalant726 marked this conversation as resolved.
Show resolved Hide resolved
2 changes: 2 additions & 0 deletions fideslog/sdk/python/_version.py
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
# type: ignore
# pylint: skip-file

# This file helps to compute a version number in source trees obtained from
# git-archive tarball (such as those provided by githubs download-from-tag
Expand Down
20 changes: 17 additions & 3 deletions fideslog/sdk/python/event.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
from typing import Dict, List, Optional
from urllib.parse import urlparse

from ...api.models.analytics_event import ALLOWED_HTTP_METHODS
from .exceptions import InvalidEventError


Expand All @@ -13,6 +14,7 @@ class AnalyticsEvent:
"""

def __init__(
# pylint:disable=too-many-locals
eastandwestwind marked this conversation as resolved.
Show resolved Hide resolved
self,
event: str,
event_created_at: datetime,
Expand All @@ -33,7 +35,7 @@ def __init__(
:param event_created_at: The UTC timestamp when the event occurred, in [ISO 8601](https://en.wikipedia.org/wiki/ISO_8601) format. Must include the UTC timezone, and represent a datetime in the past.
:param command: For events submitted as a result of running CLI commands, the name of the command that was submitted. May include the subcommand name(s).
:param docker: `True` if the command was submitted within a Docker container. Default: `False`.
:param endpoint: For events submitted as a result of making API server requests, the API endpoint path that was requested. If a fully-qualified URL is provided, only the URL path will be persisted.
:param endpoint: For events submitted as a result of making API server requests, the HTTP method and API endpoint path included on the request, delimited by a colon. Ex: `GET: /api/path`.
:param error: For events submitted as a result of running CLI commands that exit with a non-0 status code, or events submitted as a result of API server requests that respond with a non-2xx status code, the error type, without specific error details.
:param extra_data: Any additional key/value pairs that should be associated with this event.
:param flags: For events submitted as a result of running CLI commands, the flags in use when the command was submitted. Omits flag values (when they exist) by persisting only the portion of each string in this list that come before `=` or `space` characters.
Expand Down Expand Up @@ -70,8 +72,20 @@ def __init__(

self.endpoint = None
if endpoint is not None:
assert urlparse(endpoint).path != "", "endpoint must include a URL path"
self.endpoint = endpoint
endpoint_components = endpoint.split(":")
assert (
len(endpoint_components) == 2
), "endpoint must contain only the HTTP method and URL path, delimited by a colon"
endpoint_components[0] = endpoint_components[0].strip().upper()
assert (
endpoint_components[0].strip() in ALLOWED_HTTP_METHODS
), f"HTTP method must be one of {', '.join(ALLOWED_HTTP_METHODS)}"
assert (
endpoint_components[1].strip()
== urlparse(endpoint_components[1].strip()).path
), "endpoint must contain only the URL path"
endpoint_with_uppercase_method = ":".join(endpoint_components)
self.endpoint = endpoint_with_uppercase_method
eastandwestwind marked this conversation as resolved.
Show resolved Hide resolved

self.command = command
self.docker = docker
Expand Down
3 changes: 2 additions & 1 deletion tests/api/test_models.py
Original file line number Diff line number Diff line change
Expand Up @@ -21,14 +21,15 @@ def test_full_payload() -> Generator:
"product_name": "test_product",
"production_version": "1.2.3",
"status_code": 200,
"endpoint": "https://www.example.com/path/string",
"endpoint": "GET: /path/string",
"flags": ["-f", "-y", "--test"],
"command": "apply",
"error": "Internal Server Error",
"local_host": True,
"extra_data": {"extra_value": "extra_value"},
"docker": True,
"resource_counts": {"datasets": 1, "policies": 1, "systems": 27},
"developer": True,
}


Expand Down