Skip to content

Switch to Sentry #1345

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

Merged
merged 11 commits into from
Nov 14, 2023
Merged

Switch to Sentry #1345

merged 11 commits into from
Nov 14, 2023

Conversation

korlaxxalrok
Copy link
Contributor

Summary:

We are migrating to Sentry for application observability needs.

The goal is to use it for both local development as well as (potentially) qa/staging/production/etc. environments.

This PR:

Removes and cleans up the New Relic config:

  • Removes the New Relic wrapper and package.
  • Returns the default application start process.

Adds the necessary Sentry bits:

  • Adds the Sentry package for Flask.
  • Adds and configures the Sentry SDK with useful defaults for local development.
  • Exposes common knobs and levers as env vars for tuning when running in other environments.

Updates the local development setup to make it easy to instrument for Sentry (if desired):

  • Updates the local development Makefile:
    • To create a .env file if one doesn't already exist. We need to be able to pass in the Sentry DSN (and maybe other config), and putting this into .env is best.
    • To start delphi_web_epidata with --env-file to load vars from .env

Adds comments and README updates to help with a basic Sentry setup in:

  • .env.example
  • epidata_development.md

- Delete the wrapper file
- Don't copy the wrapper into the Docker image
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
- 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.
Copy link
Collaborator

@melange396 melange396 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

a few tiny things, but otherwise looks groovy!

@korlaxxalrok
Copy link
Contributor Author

korlaxxalrok commented Nov 10, 2023

I want to work on this a bit more so converting to draft for a little bit.

@korlaxxalrok korlaxxalrok marked this pull request as draft November 10, 2023 19:06
@korlaxxalrok korlaxxalrok force-pushed the remove-newrelic-add-sentry branch from 1aab7a4 to b7be446 Compare November 14, 2023 13:40
- 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 `=`)
Copy link

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
0.0% 0.0% Duplication

@korlaxxalrok korlaxxalrok marked this pull request as ready for review November 14, 2023 22:46
Copy link
Collaborator

@melange396 melange396 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

beautiful!

@korlaxxalrok korlaxxalrok merged commit f41e1ee into dev Nov 14, 2023
@korlaxxalrok korlaxxalrok deleted the remove-newrelic-add-sentry branch November 14, 2023 23:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants