Always run container with single Uvicorn process (#503)

Previously, we'd run our application container with uvicorn in development, but gunicorn with multiple workers in production. This made the app a little more complex when it came to metrics collection, and we have k8s to manage scaling. So, with this commit, we remove gunicorn and now always run the application with a single process. With removing Gunicorn, we also had to do some tweaking to our logging and application settings.
mozilla-it · Jan 10, 2023 · aeb42dc · aeb42dc
1 parent f710b07
commit aeb42dc
Show file tree

Hide file tree

Showing 13 changed files with 64 additions and 277 deletions.
diff --git a/asgi.py b/asgi.py
@@ -0,0 +1,22 @@
+import uvicorn
+
+from ctms.app import get_settings
+from ctms.log import configure_logging
+
+settings = get_settings()
+
+
+if __name__ == "__main__":
+    logging_config = configure_logging(
+        settings.use_mozlog, settings.logging_level.name, settings.log_sqlalchemy
+    )
+
+    server = uvicorn.Server(
+        uvicorn.Config(
+            "ctms.app:app",
+            host=settings.host,
+            port=settings.port,
+            log_config=logging_config,
+        )
+    )
+    server.run()
diff --git a/ctms/app.py b/ctms/app.py
@@ -57,13 +57,8 @@
     StripeIngestUnknownObjectError,
     ingest_stripe_object,
 )
-from .log import configure_logging, context_from_request, get_log_line
-from .metrics import (
-    emit_response_metrics,
-    get_metrics_reporting_registry,
-    init_metrics,
-    init_metrics_labels,
-)
+from .log import context_from_request, get_log_line
+from .metrics import emit_response_metrics, init_metrics, init_metrics_labels
 from .models import Email, StripeCustomer
 from .monitor import check_database, get_version
 from .schemas import (
@@ -99,6 +94,9 @@
 SessionLocal = None
 METRICS_REGISTRY = CollectorRegistry()
 METRICS = None
+
+# We could use the default prometheus_client.REGISTRY, but it makes tests
+# easier to write if it is possible to replace the registry with a fresh one.
 get_metrics_registry = lambda: METRICS_REGISTRY
 get_metrics = lambda: METRICS
 oauth2_scheme = OAuth2ClientCredentials(tokenUrl="token")
@@ -119,8 +117,6 @@ def get_settings() -> config.Settings:
 @app.on_event("startup")
 def startup_event():  # pragma: no cover
     global SessionLocal, METRICS  # pylint:disable = W0603
-    settings = get_settings()
-    configure_logging(settings.use_mozlog, settings.logging_level.name)
     _, SessionLocal = get_db_engine(get_settings())
     METRICS = init_metrics(METRICS_REGISTRY)
     init_metrics_labels(SessionLocal(), app, METRICS)
@@ -915,7 +911,7 @@ def metrics(request: Request):
     if agent.startswith("Prometheus/"):
         request.state.log_context["trivial_code"] = 200
     headers = {"Content-Type": CONTENT_TYPE_LATEST}
-    registry = get_metrics_reporting_registry(get_metrics_registry())
+    registry = get_metrics_registry()
     return Response(generate_latest(registry), status_code=200, headers=headers)
 
 

diff --git a/ctms/config.py b/ctms/config.py
@@ -3,7 +3,7 @@
 from enum import Enum
 from typing import Optional
 
-from pydantic import BaseSettings, DirectoryPath, PostgresDsn
+from pydantic import BaseSettings, PostgresDsn
 
 # If primary email matches, then add trace to logs
 re_trace_email = re.compile(r".*\+trace-me-mozilla-.*@.*")
@@ -27,12 +27,13 @@ class Settings(BaseSettings):
     token_expiration: timedelta = timedelta(minutes=60)
     server_prefix: str = "http://localhost:8000"
     use_mozlog: bool = True
+    log_sqlalchemy: bool = False
     logging_level: LogLevel = LogLevel.INFO
     sentry_debug: bool = False
 
     fastapi_env: Optional[str] = None
-    is_gunicorn: bool = False
-    prometheus_multiproc_dir: Optional[DirectoryPath] = None
+    host: str = "0.0.0.0"
+    port: int = 8000
 
     pubsub_audience: Optional[str] = None
     pubsub_email: Optional[str] = None
@@ -68,8 +69,8 @@ class Config:
 
         fields = {
             "fastapi_env": {"env": "fastapi_env"},
-            "is_gunicorn": {"env": "is_gunicorn"},
-            "prometheus_multiproc_dir": {"env": "prometheus_multiproc_dir"},
+            "host": {"env": "host"},
+            "port": {"env": "port"},
         }
 
 

diff --git a/ctms/log.py b/ctms/log.py
@@ -5,24 +5,21 @@
 from typing import Any, Dict, List, Optional
 
 import structlog
-import uvicorn
 from fastapi import Request
 from starlette.routing import Match
 from structlog.types import Processor
 
 
 def configure_logging(
     use_mozlog: bool = True, logging_level: str = "INFO", log_sqlalchemy: bool = False
-) -> None:
+) -> dict:
     """Configure Python logging.
 
     :param use_mozlog: If True, use MozLog format, appropriate for deployments.
         If False, format logs for human consumption.
     :param logging_level: The logging level, such as DEBUG or INFO.
     :param log_sqlalchemy: Include SQLAlchemy engine logs, such as SQL statements
     """
-    # Processors used for logs generated by stdlib's logging
-    uvicorn_formatters = uvicorn.config.LOGGING_CONFIG["formatters"]
 
     if use_mozlog:
         structlog_fmt_prep: Processor = structlog.stdlib.render_to_log_kwargs
@@ -47,67 +44,37 @@ def configure_logging(
                 "()": "dockerflow.logging.JsonLogFormatter",
                 "logger_name": "ctms",
             },
-            "uvicorn_access": uvicorn_formatters["access"],
-            "uvicorn_default": uvicorn_formatters["default"],
         },
         "handlers": {
             "humans": {
                 "class": "logging.StreamHandler",
                 "formatter": "dev_console",
-                "level": "DEBUG",
+                "level": logging.DEBUG,
+            },
+            "null": {
+                "class": "logging.NullHandler",
             },
             "mozlog": {
                 "class": "logging.StreamHandler",
                 "formatter": "mozlog_json",
-                "level": "DEBUG",
-            },
-            "uvicorn.access": {
-                "class": "logging.StreamHandler",
-                "formatter": "uvicorn_access",
-                "level": "INFO",
-            },
-            "uvicorn.default": {
-                "class": "logging.StreamHandler",
-                "formatter": "uvicorn_default",
-                "level": "INFO",
+                "level": logging.DEBUG,
             },
         },
+        "root": {
+            "handlers": ["mozlog" if use_mozlog else "humans"],
+            "level": logging_level,
+        },
         "loggers": {
-            "alembic": {
-                "propagate": False,
-                "handlers": ["mozlog" if use_mozlog else "humans"],
-                "level": logging_level,
-            },
-            "ctms": {
-                "propagate": False,
-                "handlers": ["mozlog" if use_mozlog else "humans"],
-                "level": logging_level,
-            },
-            "uvicorn": {
-                "handlers": ["mozlog" if use_mozlog else "uvicorn.default"],
-                "level": logging_level,
-                "propagate": False,
-            },
-            "uvicorn.error": {
-                "handlers": ["mozlog" if use_mozlog else "uvicorn.default"],
-                "level": logging_level,
-                "propagate": False,
-            },
-            "uvicorn.access": {
-                "handlers": ["mozlog" if use_mozlog else "uvicorn.access"],
-                "level": "WARNING",  # was INFO, but duplicates ctms.web
-                "propagate": False,
-            },
+            "alembic": {"level": logging_level},
+            "ctms": {"level": logging_level},
+            "uvicorn": {"level": logging_level},
+            "uvicorn.access": {"handlers": ["null"], "propagate": False},
             "sqlalchemy.engine": {
-                "handlers": ["mozlog" if use_mozlog else "uvicorn.default"],
-                "level": logging_level if log_sqlalchemy else "WARNING",
+                "handlers": ["mozlog" if use_mozlog else "humans"],
+                "level": logging_level if log_sqlalchemy else logging.WARNING,
                 "propagate": False,
             },
         },
-        "root": {
-            "handlers": ["mozlog" if use_mozlog else "humans"],
-            "level": "WARNING",
-        },
     }
     logging.config.dictConfig(logging_config)
 
@@ -129,6 +96,7 @@ def configure_logging(
         wrapper_class=structlog.stdlib.BoundLogger,
         cache_logger_on_first_use=True,
     )
+    return logging_config
 
 
 def context_from_request(request: Request) -> Dict:

diff --git a/ctms/metrics.py b/ctms/metrics.py
@@ -5,13 +5,10 @@
 
 from fastapi import FastAPI
 from prometheus_client import CollectorRegistry, Counter, Histogram
-from prometheus_client.multiprocess import MultiProcessCollector
 from prometheus_client.utils import INF
-from pydantic import ValidationError
 from sqlalchemy.orm import Session
 from starlette.routing import Route
 
-from ctms import config
 from ctms.crud import get_active_api_client_ids
 
 METRICS_PARAMS: dict[str, tuple[Type[Counter] | Type[Histogram], dict]] = {
@@ -60,37 +57,6 @@
 }
 
 
-def get_metrics_reporting_registry(
-    process_registry: CollectorRegistry,
-) -> CollectorRegistry:
-    """
-    Get the metrics registry for reporting metrics.
-
-    If we're running under gunicorn, then each worker has its own process and
-    its own process collector. For reporting, we need a fresh registry with a
-    multiprocess collector that points to the metrics folder (created empty at
-    startup, with a database file for each process and metric type). It will
-    use the databases in this folder to generate combined metrics across the
-    processes, and will not double-count the reporting process's metrics.
-
-    If we're not running under gunicorn, then return the passed per-process
-    registry, which is the only metrics registry. In the single-process case,
-    We could use the default prometheus_client.REGISTRY, but it makes tests
-    easier to write if it is possible to replace the registry with a fresh one.
-    """
-    try:
-        settings = config.Settings()
-        prometheus_multiproc_dir = settings.prometheus_multiproc_dir
-    except ValidationError:
-        prometheus_multiproc_dir = None
-
-    if prometheus_multiproc_dir:
-        registry = CollectorRegistry()
-        MultiProcessCollector(registry, path=prometheus_multiproc_dir)
-        return registry
-    return process_registry
-
-
 def init_metrics(registry: CollectorRegistry) -> dict[str, Counter | Histogram]:
     """Initialize the metrics with the registry."""
     metrics = {}

diff --git a/docker/Dockerfile b/docker/Dockerfile
@@ -17,7 +17,7 @@ ENV PYTHONPATH=/app \
     VENV_PATH="/opt/pysetup/.venv" \
     PORT=8000
 
-ENV PATH="$POETRY_HOME/bin:$VENV_PATH/bin:$PATH"
+ENV PATH="$VENV_PATH/bin:$POETRY_HOME/bin:$PATH"
 
 # Set up user and group
 ARG userid=10001
@@ -68,10 +68,6 @@ USER app
 COPY --from=builder-base $POETRY_HOME $POETRY_HOME
 COPY --from=builder-base $PYSETUP_PATH $PYSETUP_PATH
 
-# Copying in our entrypoint
-COPY --chown=app:app ./docker/docker-entrypoint.sh /docker-entrypoint.sh
-RUN chmod +x /docker-entrypoint.sh
-
 # venv already has runtime deps installed we get a quicker install
 WORKDIR $PYSETUP_PATH
 RUN poetry install --no-root
@@ -80,25 +76,17 @@ WORKDIR /app
 COPY --chown=app:app . .
 
 EXPOSE $PORT
-ENTRYPOINT ["/docker-entrypoint.sh"]
-CMD uvicorn ctms.app:app --reload --host=0.0.0.0 --port=$PORT
+CMD ["python", "asgi.py"]
 
 # 'production' stage uses the clean 'python-base' stage and copyies
 # in only our runtime deps that were installed in the 'builder-base'
 FROM python-base as production
-ENV FASTAPI_ENV=production \
-    IS_GUNICORN=1 \
-    PROMETHEUS_MULTIPROC=1
+ENV FASTAPI_ENV=production
 
 COPY --from=builder-base $VENV_PATH $VENV_PATH
-COPY ./docker/gunicorn_conf.py /gunicorn_conf.py
-
-COPY ./docker/docker-entrypoint.sh /docker-entrypoint.sh
-RUN chmod +x /docker-entrypoint.sh
 
-COPY . /app
 WORKDIR /app
+COPY --chown=app:app . .
 
 EXPOSE $PORT
-ENTRYPOINT ["/docker-entrypoint.sh"]
-CMD gunicorn -k uvicorn.workers.UvicornWorker -c /gunicorn_conf.py ctms.app:app
+CMD ["python", "asgi.py"]
diff --git a/docker/docker-entrypoint.sh b/docker/docker-entrypoint.sh