From 3603d7f05bea80a82e14f36c70de2780a3f14145 Mon Sep 17 00:00:00 2001 From: Brian Clark Date: Mon, 30 Oct 2023 12:44:25 -0400 Subject: [PATCH 01/11] Remove New Relic startup wrapper - Delete the wrapper file - Don't copy the wrapper into the Docker image --- devops/Dockerfile | 4 +--- devops/start_wrapper.sh | 10 ---------- 2 files changed, 1 insertion(+), 13 deletions(-) delete mode 100644 devops/start_wrapper.sh diff --git a/devops/Dockerfile b/devops/Dockerfile index 97dc0e2c8..3c5ff2672 100644 --- a/devops/Dockerfile +++ b/devops/Dockerfile @@ -7,7 +7,6 @@ FROM tiangolo/meinheld-gunicorn:python3.8 LABEL org.opencontainers.image.source=https://github.com/cmu-delphi/delphi-epidata COPY ./devops/gunicorn_conf.py /app -COPY ./devops/start_wrapper.sh / RUN mkdir -p /app/delphi/epidata COPY ./src/server /app/delphi/epidata/server COPY ./src/common /app/delphi/epidata/common @@ -18,7 +17,6 @@ COPY requirements.api.txt /app/requirements_also.txt RUN ln -s -f /usr/share/zoneinfo/America/New_York /etc/localtime \ && rm -rf /app/delphi/epidata/__pycache__ \ && chmod -R o+r /app/delphi/epidata \ - && chmod 755 /start_wrapper.sh \ && pip install --no-cache-dir -r /tmp/requirements.txt -r requirements_also.txt # the file /tmp/requirements.txt is created in the parent docker definition. (see: # https://github.com/tiangolo/meinheld-gunicorn-docker/blob/master/docker-images/python3.8.dockerfile#L5 ) @@ -28,4 +26,4 @@ RUN ln -s -f /usr/share/zoneinfo/America/New_York /etc/localtime \ ENV PYTHONUNBUFFERED 1 ENTRYPOINT [ "/entrypoint.sh" ] -CMD [ "/start_wrapper.sh" ] +CMD [ "/start.sh" ] diff --git a/devops/start_wrapper.sh b/devops/start_wrapper.sh deleted file mode 100644 index a5192d6e3..000000000 --- a/devops/start_wrapper.sh +++ /dev/null @@ -1,10 +0,0 @@ -#!/usr/bin/env sh -set -e - -# If a New Relic license key is found then we start with custom New Relic -# commands, otherwise we start via start.sh. -if [ -z "${NEW_RELIC_LICENSE_KEY}" ]; then - sh /start.sh -else - newrelic-admin run-program /start.sh -fi From b334f3af55a3a22f0768449137167b4bbdf036e1 Mon Sep 17 00:00:00 2001 From: Brian Clark Date: Mon, 30 Oct 2023 12:54:00 -0400 Subject: [PATCH 02/11] Remove newrelic/add sentry packages --- requirements.api.txt | 2 +- requirements.dev.txt | 1 + 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/requirements.api.txt b/requirements.api.txt index d2c47f64b..841cf42cb 100644 --- a/requirements.api.txt +++ b/requirements.api.txt @@ -5,7 +5,6 @@ Flask-Limiter==3.3.0 jinja2==3.0.3 more_itertools==8.4.0 mysqlclient==2.1.1 -newrelic orjson==3.4.7 pandas==1.2.3 python-dotenv==0.15.0 @@ -18,3 +17,4 @@ structlog==22.1.0 tenacity==7.0.0 typing-extensions werkzeug==2.2.3 +sentry-sdk[flask] diff --git a/requirements.dev.txt b/requirements.dev.txt index a92efdf8d..5442c0618 100644 --- a/requirements.dev.txt +++ b/requirements.dev.txt @@ -21,3 +21,4 @@ selenium==4.7.2 sqlalchemy-stubs>=0.3 tenacity==7.0.0 xlrd==2.0.1 +sentry-sdk[flask] From fba6dd52aca7b8c348c7de939acc9476a057a8dd Mon Sep 17 00:00:00 2001 From: Brian Clark Date: Mon, 30 Oct 2023 13:06:29 -0400 Subject: [PATCH 03/11] Add Sentry SDK config Adds imports for - `os` for access to the system environment - `sentry_sdk` for Sentry goodness Adds a default Sentry config that is useful and verbose for development - This will only be enabled if a Sentry DSN is supplied, otherwise it is ignored - These defaults can/will be turned down a bit for production via our config repo --- src/server/main.py | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/src/server/main.py b/src/server/main.py index a91a91ee2..d6332a478 100644 --- a/src/server/main.py +++ b/src/server/main.py @@ -1,5 +1,7 @@ +import os import pathlib import logging +import sentry_sdk from typing import Dict, Callable from flask import request, send_file, Response, send_from_directory, jsonify @@ -13,6 +15,18 @@ from .endpoints.admin import bp as admin_bp, enable_admin from ._limiter import limiter, apply_limit +SENTRY_DSN = os.environ.get('SENTRY_DSN') +if SENTRY_DSN: + sentry_sdk.init( + dsn=SENTRY_DSN, + traces_sample_rate=float(os.environ.get('SENTRY_TRACES_SAMPLE_RATE', 1.0)), + profiles_sample_rate=float(os.environ.get('SENTRY_PROFILES_SAMPLE_RATE', 1.0)), + environment=str(os.environ.get('SENTRY_ENVIRONMENT', 'development')), + debug=str(os.environ.get('SENTRY_DEBUG', 'True')), + attach_stacktrace=str(os.environ.get('SENTRY_ATTACH_STACKTRACE', 'True')) + ) + + __all__ = ["app"] logger = get_structured_logger("webapp_main") From 0593e120737737a24af14e2427d3cf1cd9b70cf7 Mon Sep 17 00:00:00 2001 From: Brian Clark Date: Tue, 7 Nov 2023 10:36:30 -0500 Subject: [PATCH 04/11] Add comment --- .env.example | 3 +++ 1 file changed, 3 insertions(+) diff --git a/.env.example b/.env.example index 51cac1ae4..0ffe2a1e8 100644 --- a/.env.example +++ b/.env.example @@ -4,3 +4,6 @@ FLASK_SECRET=abc #API_KEY_REQUIRED_STARTING_AT=2021-07-30 API_KEY_ADMIN_PASSWORD=abc API_KEY_REGISTER_WEBHOOK_TOKEN=abc + +# Sentry +# If setting a Sentry DSN, note that the URL should NOT be quoted! From 824f9a9f7397f0a1b9171d3f73e36a6f5a54fce7 Mon Sep 17 00:00:00 2001 From: Brian Clark Date: Wed, 8 Nov 2023 14:53:18 -0500 Subject: [PATCH 05/11] Update Makefile to create an .env file - Updates our Makefile with a new target to create an empty .env file if one does not already exist. If one already exists it won't be altered. - Updates the `web:` target to use the .env file when starting the container. --- dev/local/Makefile | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/dev/local/Makefile b/dev/local/Makefile index 84c308257..c4318d99d 100644 --- a/dev/local/Makefile +++ b/dev/local/Makefile @@ -77,6 +77,7 @@ LOG_REDIS:=delphi_redis_instance_$(NOW).log WEB_CONTAINER_ID:=$(shell docker ps -q --filter 'name=delphi_web_epidata') DATABASE_CONTAINER_ID:=$(shell docker ps -q --filter 'name=delphi_database_epidata') REDIS_CONTAINER_ID:=$(shell docker ps -q --filter 'name=delphi_redis') +ENV_FILE:=repos/delphi/delphi-epidata/.env M1= ifeq ($(shell uname -smp), Darwin arm64 arm) @@ -106,6 +107,7 @@ web: @# see https://github.com/tiangolo/meinheld-gunicorn-docker#module_name @docker run --rm -p 127.0.0.1:10080:80 \ $(M1) \ + --env-file $(ENV_FILE) \ --env "MODULE_NAME=delphi.epidata.server.main" \ --env "SQLALCHEMY_DATABASE_URI=$(sqlalchemy_uri)" \ --env "FLASK_SECRET=abc" --env "FLASK_PREFIX=/epidata" --env "LOG_DEBUG" \ @@ -170,7 +172,7 @@ redis: --name delphi_redis delphi_redis >$(LOG_REDIS) 2>&1 & .PHONY=all -all: db web py redis +all: env db web py redis .PHONY=test test: @@ -211,3 +213,7 @@ sql: .PHONY=clean clean: @docker images -f "dangling=true" -q | xargs docker rmi >/dev/null 2>&1 + +.PHONY=env +env: + @touch $(ENV_FILE) From 05d5536995c1158126791337ff1031c94f145ccf Mon Sep 17 00:00:00 2001 From: Brian Clark Date: Thu, 9 Nov 2023 14:18:17 -0500 Subject: [PATCH 06/11] Update the README with Sentry information --- docs/epidata_development.md | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/docs/epidata_development.md b/docs/epidata_development.md index 29a0fdde7..149a086d8 100644 --- a/docs/epidata_development.md +++ b/docs/epidata_development.md @@ -388,3 +388,13 @@ The command above maps two local directories into the container: - `/repos/delphi/delphi-epidata/src`: Just the source code, which forms the container's `delphi.epidata` python package. +## instrumentation with Sentry + +Delphi uses [Sentry](https://sentry.io/welcome/) in production for debugging, APM, and other observability purposes. You can instrument your local environment if you want to take advantage of Sentry's features during the development process. In most cases this option is available to internal Delphi team members only. + +The bare minimum to set up instrumentation is to supply the DSN for the [epidata-api](https://cmu-delphi.sentry.io/projects/epidata-api/?project=4506123377442816) Sentry project to the application environment. + +- You can get the DSN from the Sentry [project's keys config](https://cmu-delphi.sentry.io/settings/projects/epidata-api/keys/), or by asking someone in the prodsys, DevOps, or sysadmin space. +- Once you have the DSN, add it to your local `.env` file and rebuild your containers to start sending telemetry to Sentry. + +Additional internal documentation for Sentry can be found [here](https://bookstack.delphi.cmu.edu/books/systems-handbook/page/sentry). \ No newline at end of file From 2f18b196d917bd9d5184ee33f39d9e47e8042948 Mon Sep 17 00:00:00 2001 From: Brian Clark Date: Thu, 9 Nov 2023 16:16:57 -0500 Subject: [PATCH 07/11] Add newline --- docs/epidata_development.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/epidata_development.md b/docs/epidata_development.md index 149a086d8..c8c35e11f 100644 --- a/docs/epidata_development.md +++ b/docs/epidata_development.md @@ -397,4 +397,4 @@ The bare minimum to set up instrumentation is to supply the DSN for the [epidata - You can get the DSN from the Sentry [project's keys config](https://cmu-delphi.sentry.io/settings/projects/epidata-api/keys/), or by asking someone in the prodsys, DevOps, or sysadmin space. - Once you have the DSN, add it to your local `.env` file and rebuild your containers to start sending telemetry to Sentry. -Additional internal documentation for Sentry can be found [here](https://bookstack.delphi.cmu.edu/books/systems-handbook/page/sentry). \ No newline at end of file +Additional internal documentation for Sentry can be found [here](https://bookstack.delphi.cmu.edu/books/systems-handbook/page/sentry). From 2fc8e196a6f3ab16296ad514d5c7385702930dd4 Mon Sep 17 00:00:00 2001 From: Brian Clark Date: Fri, 10 Nov 2023 13:48:36 -0500 Subject: [PATCH 08/11] Update requirements.dev.txt Co-authored-by: melange396 --- requirements.dev.txt | 1 - 1 file changed, 1 deletion(-) diff --git a/requirements.dev.txt b/requirements.dev.txt index 5442c0618..a92efdf8d 100644 --- a/requirements.dev.txt +++ b/requirements.dev.txt @@ -21,4 +21,3 @@ selenium==4.7.2 sqlalchemy-stubs>=0.3 tenacity==7.0.0 xlrd==2.0.1 -sentry-sdk[flask] From b7be4464e705135041a952d3382dfbae26f2f079 Mon Sep 17 00:00:00 2001 From: Brian Clark Date: Mon, 13 Nov 2023 09:03:24 -0500 Subject: [PATCH 09/11] Place in alphabetical order --- requirements.api.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/requirements.api.txt b/requirements.api.txt index 841cf42cb..b00eb3e06 100644 --- a/requirements.api.txt +++ b/requirements.api.txt @@ -12,9 +12,9 @@ pyyaml redis==3.5.3 requests==2.31.0 scipy==1.10.0 +sentry-sdk[flask] SQLAlchemy==1.4.40 structlog==22.1.0 tenacity==7.0.0 typing-extensions werkzeug==2.2.3 -sentry-sdk[flask] From 254692a6c233630ca5ea5336b7d258922b26cf10 Mon Sep 17 00:00:00 2001 From: Brian Clark Date: Tue, 14 Nov 2023 14:22:38 -0500 Subject: [PATCH 10/11] Updates to Sentry SDK config and env vars - Set `debug` and `attach_stacktrace` to `False` - Reordered a bit - Added a decent way to make an env var work as a boolean - Better formatting (spaces around `=`) --- src/server/main.py | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/src/server/main.py b/src/server/main.py index d6332a478..2ec07e5a5 100644 --- a/src/server/main.py +++ b/src/server/main.py @@ -18,12 +18,12 @@ SENTRY_DSN = os.environ.get('SENTRY_DSN') if SENTRY_DSN: sentry_sdk.init( - dsn=SENTRY_DSN, - traces_sample_rate=float(os.environ.get('SENTRY_TRACES_SAMPLE_RATE', 1.0)), - profiles_sample_rate=float(os.environ.get('SENTRY_PROFILES_SAMPLE_RATE', 1.0)), - environment=str(os.environ.get('SENTRY_ENVIRONMENT', 'development')), - debug=str(os.environ.get('SENTRY_DEBUG', 'True')), - attach_stacktrace=str(os.environ.get('SENTRY_ATTACH_STACKTRACE', 'True')) + dsn = SENTRY_DSN, + environment = os.environ.get('SENTRY_ENVIRONMENT', 'development'), + profiles_sample_rate = float(os.environ.get('SENTRY_PROFILES_SAMPLE_RATE', 1.0)), + traces_sample_rate = float(os.environ.get('SENTRY_TRACES_SAMPLE_RATE', 1.0)), + attach_stacktrace = os.environ.get('SENTRY_ATTACH_STACKTRACE', 'False').lower() in ('true', '1', 't'), + debug = os.environ.get('SENTRY_DEBUG', 'False').lower() in ('true', '1', 't') ) From 1a0df5d603a586927c28602c3bbdd43e0d27b4eb Mon Sep 17 00:00:00 2001 From: Brian Clark Date: Tue, 14 Nov 2023 17:33:17 -0500 Subject: [PATCH 11/11] Move .env file creation to web target --- dev/local/Makefile | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/dev/local/Makefile b/dev/local/Makefile index c4318d99d..d0854a064 100644 --- a/dev/local/Makefile +++ b/dev/local/Makefile @@ -105,6 +105,7 @@ web: @# Run the web server @# MODULE_NAME specifies the location of the `app` variable, the actual WSGI application object to run. @# see https://github.com/tiangolo/meinheld-gunicorn-docker#module_name + @touch $(ENV_FILE) @docker run --rm -p 127.0.0.1:10080:80 \ $(M1) \ --env-file $(ENV_FILE) \ @@ -172,7 +173,7 @@ redis: --name delphi_redis delphi_redis >$(LOG_REDIS) 2>&1 & .PHONY=all -all: env db web py redis +all: db web py redis .PHONY=test test: @@ -213,7 +214,3 @@ sql: .PHONY=clean clean: @docker images -f "dangling=true" -q | xargs docker rmi >/dev/null 2>&1 - -.PHONY=env -env: - @touch $(ENV_FILE)