Fix middleware runtime error and release prep fixes (#130)

* Temporarily use fork for starlette 0.21 release

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

See:
https://github.com/encode/starlette/pull/1710
https://github.com/encode/starlette/pull/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:

https://github.com/tkem/cachetools/issues/80

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

* Lint

* Changelog

CHANGELOG.md

@@ -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]
 

deployment/terraform/resources/functions.tf

@@ -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 = ["*"]

pccommon/pccommon/tables.py

@@ -1,3 +1,4 @@
+from threading import Lock
 from typing import (
     Any,
     Callable,
@@ -58,6 +59,7 @@ class TableService:
         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:
@@ -187,7 +189,7 @@ class ModelTableService(Generic[M], TableService):
                 }
             )
 
-    @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:

pccommon/pccommon/tracing.py

@@ -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),
@@ -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)
             )
@@ -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)

pccommon/setup.py

@@ -4,7 +4,12 @@ from setuptools import find_packages, setup
 
 # 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",

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
 

pcfuncs/animation/animation.py

@@ -14,7 +14,6 @@ from mercantile import Tile, tiles
 from PIL.Image import Image as PILImage
 from pyproj import CRS
 
-
 from .constants import MAX_TILE_COUNT
 from .settings import AnimationSettings
 

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

pctiler/pctiler/main.py

@@ -7,7 +7,6 @@ from fastapi import FastAPI, Request, Response
 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 (
@@ -18,6 +17,7 @@ from titiler.core.middleware import (
 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,

requirements-dev.txt

@@ -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