Skip to content

Commit

Permalink
Fix middleware runtime error and release prep fixes (#130)
Browse files Browse the repository at this point in the history
* Temporarily use fork for starlette 0.21 release

The 0.21 release resolves a frequent error on our fastapi version.

See:
encode/starlette#1710
encode/starlette#1715

* Disable FTP as function app deploy option

Security controls

* Trace request attributes before invoking middleware

If an exception is raised in subsequent middlewares, added trace
attributes will still be logged to Azure. This allows us to find
requests that fail in the logs.

* Make config cache thread safe

cachetools cache is not thread safe and there were frequent exceptions
logged indicating that cache updates during async calls were failing
with key errors similar to those described in:

tkem/cachetools#80

Add a lock per table instance synchronizes cache updates across threads
in.

* Lint

* Changelog
  • Loading branch information
mmcfarland committed Oct 25, 2022
1 parent 72be508 commit b4912be
Show file tree
Hide file tree
Showing 10 changed files with 30 additions and 11 deletions.
3 changes: 3 additions & 0 deletions CHANGELOG.md
Expand Up @@ -20,6 +20,9 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
### Fixed

- Configure CORS correctly at the nginx-ingress level [#127](https://github.com/microsoft/planetary-computer-apis/pull/127)
- Make config cache access thread safe to prevent key errors [#130](https://github.com/microsoft/planetary-computer-apis/pull/130)
- Upgrade starlette (via fork) to prevent middleware errors [#130](https://github.com/microsoft/planetary-computer-apis/pull/130)
- Better request tracing [#130](https://github.com/microsoft/planetary-computer-apis/pull/130)

## [2022.3.0]

Expand Down
1 change: 1 addition & 0 deletions deployment/terraform/resources/functions.tf
Expand Up @@ -48,6 +48,7 @@ resource "azurerm_function_app" "pcfuncs" {
site_config {
linux_fx_version = "PYTHON|3.8"
use_32_bit_worker_process = false
ftps_state = "Disabled"

cors {
allowed_origins = ["*"]
Expand Down
4 changes: 3 additions & 1 deletion pccommon/pccommon/tables.py
@@ -1,3 +1,4 @@
from threading import Lock
from typing import (
Any,
Callable,
Expand Down Expand Up @@ -58,6 +59,7 @@ def __init__(
self._service_client: Optional[TableServiceClient] = None
self._table_client: Optional[TableClient] = None
self._cache: Cache = TTLCache(maxsize=1024, ttl=ttl or DEFAULT_TTL)
self._cache_lock: Lock = Lock()

def _ensure_table_client(self) -> None:
if not self._table_client:
Expand Down Expand Up @@ -187,7 +189,7 @@ def update(self, partition_key: str, row_key: str, entity: M) -> None:
}
)

@cachedmethod(cache=lambda self: self._cache)
@cachedmethod(cache=lambda self: self._cache, lock=lambda self: self._cache_lock)
def get(self, partition_key: str, row_key: str) -> Optional[M]:
with self as table_client:
try:
Expand Down
13 changes: 8 additions & 5 deletions pccommon/pccommon/tracing.py
Expand Up @@ -62,8 +62,7 @@ async def trace_request(
# are slow.
request.state.parent_span = span

response = await call_next(request)

# Add request dimensions to the trace prior to calling the next middleware
tracer.add_attribute_to_current_span(
attribute_key="ref_id",
attribute_value=request.headers.get(X_AZURE_REF),
Expand All @@ -76,9 +75,6 @@ async def trace_request(
attribute_key="request_ip",
attribute_value=get_request_ip(request),
)
tracer.add_attribute_to_current_span(
attribute_key=HTTP_STATUS_CODE, attribute_value=response.status_code
)
tracer.add_attribute_to_current_span(
attribute_key=HTTP_METHOD, attribute_value=str(request.method)
)
Expand All @@ -103,6 +99,13 @@ async def trace_request(
attribute_key="item", attribute_value=item_id
)

# Call next middleware
response = await call_next(request)

# Include response dimensions in the trace
tracer.add_attribute_to_current_span(
attribute_key=HTTP_STATUS_CODE, attribute_value=response.status_code
)
return response
else:
return await call_next(request)
Expand Down
7 changes: 6 additions & 1 deletion pccommon/setup.py
Expand Up @@ -4,7 +4,12 @@

# Runtime requirements.
inst_reqs = [
"fastapi>=0.75.2",
# --->
# TODO: restore fastapi release install after starlette dep upgraded to >= 0.21.0
# "fastapi>=0.75.2",
"fastapi @ git+https://github.com/mmcfarland/fastapi/@982e7caf086bffeace8554da6d69e5f3082f14a3#egg=fastapi",
"starlette>=0.21.0,<0.22.0",
# <---
"opencensus-ext-azure==1.0.8",
"opencensus-ext-logging==0.1.0",
"orjson==3.5.2",
Expand Down
3 changes: 3 additions & 0 deletions pcfuncs/Dockerfile
@@ -1,5 +1,8 @@
FROM mcr.microsoft.com/azure-functions/python:4-python3.8

# git required for pip installs from git
RUN apt update && apt install -y git

ENV AzureWebJobsScriptRoot=/home/site/wwwroot \
AzureFunctionsJobHost__Logging__Console__IsEnabled=true

Expand Down
1 change: 0 additions & 1 deletion pcfuncs/animation/animation.py
Expand Up @@ -14,7 +14,6 @@
from PIL.Image import Image as PILImage
from pyproj import CRS


from .constants import MAX_TILE_COUNT
from .settings import AnimationSettings

Expand Down
3 changes: 3 additions & 0 deletions pctiler/Dockerfile
@@ -1,5 +1,8 @@
FROM python:3.9-slim

# git required for pip installs from git
RUN apt update && apt install -y git

# The devops Personal Access Token for accessing
# Azure Artifacts. Note: This will be visible as
# plain text in the docker build logs. Only use your
Expand Down
2 changes: 1 addition & 1 deletion pctiler/pctiler/main.py
Expand Up @@ -7,7 +7,6 @@
from fastapi.openapi.utils import get_openapi
from morecantile.defaults import tms as defaultTileMatrices
from morecantile.models import TileMatrixSet
from pccommon.constants import X_REQUEST_ENTITY
from starlette.middleware.cors import CORSMiddleware
from titiler.core.errors import DEFAULT_STATUS_CODES, add_exception_handlers
from titiler.core.middleware import (
Expand All @@ -18,6 +17,7 @@
from titiler.mosaic.errors import MOSAIC_STATUS_CODES
from titiler.pgstac.db import close_db_connection, connect_to_db

from pccommon.constants import X_REQUEST_ENTITY
from pccommon.logging import ServiceName, init_logging
from pccommon.middleware import (
RequestTracingMiddleware,
Expand Down
4 changes: 2 additions & 2 deletions requirements-dev.txt
Expand Up @@ -6,8 +6,8 @@ openapi-spec-validator==0.3.0
cachetools<=4.2.
pytest==7.*
pytest-asyncio==0.18.*
httpx==0.19.0
httpx>=0.22.0

# Mypy types

types-cachetools
types-cachetools

0 comments on commit b4912be

Please sign in to comment.