~suligap/canonical-identity-provider:talisker-init-with-sentry-overhaul

Last commit made on 2020-05-20
Get this branch:
git clone -b talisker-init-with-sentry-overhaul https://git.launchpad.net/~suligap/canonical-identity-provider
Only Przemysław Suliga can upload to this branch. If you are Przemysław Suliga please log in for upload directions.

Branch merges

Branch information

Name:
talisker-init-with-sentry-overhaul
Repository:
lp:~suligap/canonical-identity-provider

Recent commits

99cd5ce... by Przemysław Suliga

Do not log talisker.wsgi INFO and lower to the logfile

To maintain a better backwards compatibility with pre-talisker logging
setup for now.

036147b... by Przemysław Suliga

Switch to talisker and simplify/standardise the Sentry config

1. Adds talisker as a dependency and switches the `make run` target to use
   talisker instead of gunicorn.

2. Drops the `canonical_raven` dependency along with:

- `canonical_raven.middleware.RavenContextMiddleware` django middleware
- `canonical_raven.middleware.Sentry` WSGI middleware
- `RemoveCookiesProcessor` and `SanitizeTimelineDjangoProcessor`

Both removed middlewares were adding the contents of the `timeline`
(lp:python-timeline) to the "extra" section of the message sent to Sentry.

The `SanitizeTimelineDjangoProcessor` was responsible for stripping the
contents of SQL queries containing the names of two tables: "django_session"
and "auth_user" from the timeline contents. (Other potentially sensitive
queries and their arguments were being sent to Sentry still).

`RemoveCookiesProcessor` was removing all cookies and their contents (both
from request headers and those provided by raven's django integration) from the
messages sent to Sentry. Talisker's default raven setup takes care of not
sending out the values of sensitive cookies as well as other bits of
potentially sensitive information contained in request headers for example.
(This is also why a raven upgrade was needed -- in order to have the required
raven processor).

The canonical_raven `Sentry` WSGI middleware was also responsible for sending out
ERRORs to sentry when a soft request processing timeout was exceeded
(configured using settings.HANDLER_TIMEOUT_MILLIS). Talisker provides this
functionality as well (slight differences but functionally the same -- for
example a warning is sent instead of an error), but it needs to be explicitly
switched on using an env var: TALISKER_SOFT_REQUEST_TIMEOUT (also milliseconds).

3. No longer needed settings.USE_SENTRY was removed - This is now controlled
   using the SENTRY_DSN env var or `settings.RAVEN_CONFIG['dsn']` (both work
   with talisker).

If there are reasons for keeping the `timeline` data in sentry messages, then
we can work on top of this change to re-add this on top of the talisker sentry
setup. But we should consider that:

- timelines contain entire SQL queries with potentially sensitive info in them
  (I've seen email addresses, openid identifiers, identityprovider_token.key
  which might not be secret, but still sensitive?)
- The oops reports still contain the timelines.
- This default talisker sentry setup makes this change much cleaner and results
  in a "standard django service" sentry setup, that is familiar to us already.

Lastly, this change only focuses on adding talisker and using it to take care
of raven/sentry configuration. More talisker related functionalities can be
enabled after. Specifically, we could look at logging (SSO logs to a file
currently) and re-using the statsd client provided by talisker. Sending out the
SQL queries as sentry breadcrumbs (without the contents of the query arguments)
can be enabled too, which partly replaces the timelines.

2e91bca... by Simon Davy

Put install_sql_hook in raven_config where it is properly used (not in sentry_config which kills the world).

Merged from https://code.launchpad.net/~bloodearnest/canonical-identity-provider/+git/canonical-identity-provider/+merge/383959

793ad8c... by Simon Davy

Use RAVEN_CONFIG rather than SENTRY_CONFIG

943addb... by Przemysław Suliga

Update the initial setup section of the README

Merged from https://code.launchpad.net/~suligap/canonical-identity-provider/+git/canonical-identity-provider/+merge/383874

6e18ea2... by Przemysław Suliga

Update the initial setup section of the README

More information on how to setup the container and removal of some
outdated conflicting information.

553f69f... by Simon Davy

use proper config directive to disable sql hook

Merged from https://code.launchpad.net/~bloodearnest/canonical-identity-provider/+git/canonical-identity-provider/+merge/383722

1854865... by Simon Davy

use proper config directive to disable sql hook

06bba49... by Daniel Manrique

Automatically generate and show a paper backup device when a non-paper device is added and no backup device existed.

This is controllable with a feature flag.

Merged from https://code.launchpad.net/~roadmr/canonical-identity-provider/+git/canonical-identity-provider/+merge/383650

18601c1... by Maximiliano Bertacchini

Move olstests and ols-tests-django to devel requirements.

Merged from https://code.launchpad.net/~maxiberta/canonical-identity-provider/+git/canonical-identity-provider/+merge/383717