Use new sentry-sdk and adapt usage pattern

Replaces our custom Sentry WSGI middleware with middleware shipped by
sentry-sdk. Adds sentry-sdk ASGI middleware. Our custom sentry
middleware also allowed logging slow requests to sentry. I have dropped
this for now, and if we still want this in the future, we should
probably implement this as a custom logging middleware instead, which
would automatically appear in Sentry as well.

We used to hardcode the log level for events to WARNING, and the Sentry
default is ERROR now, which I think is a good idea. I've made this
configurable using the sentry_event_level config variable.

It does not seem to be possible to connect multiple dsn's in the same
app, unless we start hacking
(https://github.com/getsentry/sentry-python/issues/198), so I dropped
the custom_dsn from the sentry error reporting plugin (which has been
broken anyway, so shouldn't be a big deal).
This commit is contained in:
mvdbeek
2021-09-28 19:32:10 +02:00
parent cb9f2570f6
commit 28511e262e
12 changed files with 111 additions and 259 deletions
+8 -10
View File
@@ -2788,18 +2788,16 @@
:Type: str
~~~~~~~~~~~~~~~~~~~~~~~~~~~
``sentry_sloreq_threshold``
~~~~~~~~~~~~~~~~~~~~~~~~~~~
~~~~~~~~~~~~~~~~~~~~~~
``sentry_event_level``
~~~~~~~~~~~~~~~~~~~~~~
:Description:
Sentry slow request logging. Requests slower than the threshold
indicated below will be sent as events to the configured Sentry
server (above, sentry_dsn). A value of '0' is disabled. For
example, you would set this to .005 to log all queries taking
longer than 5 milliseconds.
:Default: ``0.0``
:Type: float
Determines the minimum log level that will be sent as an event to
Sentry. Possible values are DEBUG, INFO, WARNING, ERROR or
CRITICAL.
:Default: ``ERROR``
:Type: str
~~~~~~~~~~~~~~~
+14 -2
View File
@@ -193,10 +193,22 @@ class GalaxyManagerApplication(MinimalManagerApp, MinimalGalaxyApplication):
self.sentry_client = None
if self.config.sentry_dsn:
event_level = self.config.sentry_event_level.upper()
assert event_level in ['DEBUG', 'INFO', 'WARNING', 'ERROR', 'CRITICAL'], f"Invalid sentry event level '{self.config.sentry.event_level}'"
def postfork_sentry_client():
import raven
self.sentry_client = raven.Client(self.config.sentry_dsn, transport=raven.transport.HTTPTransport)
import sentry_sdk
from sentry_sdk.integrations.logging import LoggingIntegration
sentry_logging = LoggingIntegration(
level=logging.INFO, # Capture info and above as breadcrumbs
event_level=getattr(logging, event_level) # Send errors as events
)
self.sentry_client = sentry_sdk.init(
self.config.sentry_dsn,
release=f"{self.config.version_major}.{self.config.version_minor}",
integrations=[sentry_logging]
)
self.application_stack.register_postfork_function(postfork_sentry_client)
+1 -10
View File
@@ -45,10 +45,7 @@ from galaxy.util.properties import (
running_from_source,
)
from galaxy.web.formatting import expand_pretty_datetime_format
from galaxy.web_stack import (
get_stack_facts,
register_postfork_function
)
from galaxy.web_stack import get_stack_facts
from ..version import VERSION_MAJOR, VERSION_MINOR
log = logging.getLogger(__name__)
@@ -1106,7 +1103,6 @@ def configure_logging(config):
"""
# Get root logger
logging.addLevelName(LOGLV_TRACE, "TRACE")
root = logging.getLogger()
# PasteScript will have already configured the logger if the
# 'loggers' section was found in the config file, otherwise we do
# some simple setup using the 'log_*' values from the config.
@@ -1129,11 +1125,6 @@ def configure_logging(config):
conf['filename'] = conf.pop('filename_template').format(**get_stack_facts(config=config))
logging_conf['handlers'][name] = conf
logging.config.dictConfig(logging_conf)
if getattr(config, "sentry_dsn", None):
from raven.handlers.logging import SentryHandler
sentry_handler = SentryHandler(config.sentry_dsn)
sentry_handler.setLevel(logging.WARN)
register_postfork_function(root.addHandler, sentry_handler)
class ConfiguresGalaxyMixin:
@@ -15,7 +15,7 @@
# verbose/user_submission, but those are not necessary to provide.
# The default Email bug reporter. By default, the standard
# configuration is taken from your galaxy.ini
# configuration is taken from your galaxy.yml
- type: email
verbose: true
user_submission: true
@@ -29,8 +29,7 @@
# directory: /tmp/reports/
# Submit error reports to sentry. If a sentry_dsn is configured in your
# galaxy.ini, then Galaxy will submit the job error to Sentry. You may supply a
# separate DSN for tool reports by supplying a ``custom_dsn`` parameter.
# galaxy.yml, then Galaxy will submit the job error to Sentry.
- type: sentry
user_submission: false
+3 -6
View File
@@ -1388,12 +1388,9 @@ galaxy:
# <project_name> -> Settings -> API Keys.
#sentry_dsn: null
# Sentry slow request logging. Requests slower than the threshold
# indicated below will be sent as events to the configured Sentry
# server (above, sentry_dsn). A value of '0' is disabled. For
# example, you would set this to .005 to log all queries taking longer
# than 5 milliseconds.
#sentry_sloreq_threshold: 0.0
# Determines the minimum log level that will be sent as an event to
# Sentry. Possible values are DEBUG, INFO, WARNING, ERROR or CRITICAL.
#sentry_event_level: ERROR
# Log to statsd Statsd is an external statistics aggregator
# (https://github.com/etsy/statsd) Enabling the following options will
+1 -1
View File
@@ -191,7 +191,7 @@ class ConditionalDependencies:
def check_fluent_logger(self):
return asbool(self.config["fluent_log"])
def check_raven(self):
def check_sentry_sdk(self):
return self.config.get("sentry_dsn", None) is not None
def check_statsd(self):
@@ -2,7 +2,7 @@
psycopg2-binary==2.8.4
mysqlclient
fluent-logger
raven
sentry-sdk
pbs_python
drmaa
statsd
@@ -1,12 +1,18 @@
"""The module describes the ``sentry`` error plugin plugin."""
import logging
try:
import sentry_sdk
except ImportError:
sentry_sdk = None
from galaxy import web
from galaxy.util import string_as_bool, unicodify
from galaxy.util import string_as_bool
from . import ErrorPlugin
log = logging.getLogger(__name__)
SENTRY_SDK_IMPORT_MESSAGE = 'The Python sentry-sdk package is required to use this feature, please install it'
ERROR_TEMPLATE = """Galaxy Job Error: {tool_id} v{tool_version}
Command Line:
@@ -32,94 +38,80 @@ class SentryPlugin(ErrorPlugin):
self.redact_user_details_in_bugreport = self.app.config.redact_user_details_in_bugreport
self.verbose = string_as_bool(kwargs.get('verbose', False))
self.user_submission = string_as_bool(kwargs.get('user_submission', False))
self.custom_dsn = kwargs.get('custom_dsn', None)
self.sentry = None
# Use the built in one by default
if hasattr(self.app, 'sentry_client'):
self.sentry = self.app.sentry_client
# if they've set a custom one, override.
if self.custom_dsn:
import raven
self.sentry = raven.Client(self.custom_dsn, transport=raven.transport.HTTPTransport)
assert sentry_sdk, SENTRY_SDK_IMPORT_MESSAGE
def submit_report(self, dataset, job, tool, **kwargs):
"""Submit the error report to sentry
"""
if self.sentry:
user = job.get_user()
extra = {
'info': job.info,
'id': job.id,
'command_line': unicodify(job.command_line),
'destination_id': unicodify(job.destination_id),
'stderr': unicodify(job.stderr),
'traceback': unicodify(job.traceback),
'exit_code': job.exit_code,
'stdout': unicodify(job.stdout),
'handler': unicodify(job.handler),
'tool_id': unicodify(job.tool_id),
'tool_version': unicodify(job.tool_version),
'tool_xml': unicodify(tool.config_file) if tool else None
}
if self.redact_user_details_in_bugreport:
extra['email'] = 'redacted'
else:
if 'email' in kwargs:
extra['email'] = unicodify(kwargs['email'])
extra = {
'info': job.info,
'id': job.id,
'command_line': job.command_line,
'destination_id': job.destination_id,
'stderr': job.stderr,
'traceback': job.traceback,
'exit_code': job.exit_code,
'stdout': job.stdout,
'handler': job.handler,
'tool_id': job.tool_id,
'tool_version': job.tool_version,
'tool_xml': tool.config_file if tool else None
}
if self.redact_user_details_in_bugreport:
extra['email'] = 'redacted'
else:
if 'email' in kwargs:
extra['email'] = kwargs['email']
# User submitted message
extra['message'] = unicodify(kwargs.get('message', ''))
# User submitted message
extra['message'] = kwargs.get('message', '')
# Construct the error message to send to sentry. The first line
# will be the issue title, everything after that becomes the
# "message"
error_message = ERROR_TEMPLATE.format(**extra)
# Construct the error message to send to sentry. The first line
# will be the issue title, everything after that becomes the
# "message"
error_message = ERROR_TEMPLATE.format(**extra)
# Update context with user information in a sentry-specific manner
context = {}
# Update context with user information in a sentry-specific manner
context = {}
# Getting the url allows us to link to the dataset info page in case
# anything is missing from this report.
try:
url = web.url_for(controller="dataset",
action="details",
dataset_id=self.app.security.encode_id(dataset.id),
qualified=True)
except AttributeError:
# The above does not work when handlers are separate from the web handlers
url = None
# Getting the url allows us to link to the dataset info page in case
# anything is missing from this report.
try:
url = web.url_for(controller="dataset",
action="show_params",
dataset_id=self.app.security.encode_id(dataset.id),
qualified=True)
except AttributeError:
# The above does not work when handlers are separate from the web handlers
url = None
if self.redact_user_details_in_bugreport:
if user:
# Opauqe identifier
context['user'] = {
'id': user.id
}
else:
if user:
# User information here also places email links + allows seeing
# a list of affected users in the tags/filtering.
context['user'] = {
'name': user.username,
'email': user.email,
}
user = job.get_user()
if self.redact_user_details_in_bugreport:
if user:
# Opaque identifier
context['user'] = {
'id': user.id
}
else:
if user:
# User information here also places email links + allows seeing
# a list of affected users in the tags/filtering.
context['user'] = {
'name': user.username,
'email': user.email,
}
context['request'] = {'url': url}
context['request'] = {'url': url}
self.sentry_client.context.merge(context)
for key, value in context.items():
sentry_sdk.set_context(key, value)
sentry_sdk.set_context('job', extra)
sentry_sdk.set_tag('tool_id', job.tool_id)
sentry_sdk.set_tag('tool_version', job.tool_version)
# Send the message, using message because
response = self.sentry_client.capture(
'raven.events.Message',
tags={
'tool_id': job.tool_id,
'tool_version': job.tool_version,
},
extra=extra,
message=unicodify(error_message),
)
return (f'Submitted bug report to Sentry. Your guru meditation number is {response}', 'success')
# Send the message, using message because
response = sentry_sdk.capture_message(error_message)
return (f'Submitted bug report to Sentry. Your guru meditation number is {response}', 'success')
__all__ = ('SentryPlugin', )
@@ -1,138 +0,0 @@
"""
raven.middleware
~~~~~~~~~~~~~~~~~~~~~~~~
:copyright: (c) 2010-2012 by the Sentry Team, see AUTHORS for more details.
:license: BSD, see LICENSE for more details.
"""
import time
try:
from raven import Client
from raven.utils.wsgi import get_current_url, get_headers, get_environ
except ImportError:
Client = None
from galaxy.web_stack import register_postfork_function
RAVEN_IMPORT_MESSAGE = ('The Python raven package is required to use this '
'feature, please install it')
class Sentry:
"""
A WSGI middleware which will attempt to capture any
uncaught exceptions and send them to Sentry.
"""
def __init__(self, application, dsn, sloreq=0):
assert Client is not None, RAVEN_IMPORT_MESSAGE
self.application = application
self.client = None
self.sloreq_threshold = sloreq
def postfork_sentry_client():
self.client = Client(dsn)
register_postfork_function(postfork_sentry_client)
def __call__(self, environ, start_response):
try:
start_time = time.time()
iterable = self.application(environ, start_response)
dt = (time.time() - start_time)
if self.sloreq_threshold and dt > self.sloreq_threshold:
self.handle_slow_request(environ, dt)
except Exception:
self.handle_exception(environ)
raise
try:
yield from iterable
except Exception:
self.handle_exception(environ)
raise
finally:
# wsgi spec requires iterable to call close if it exists
# see http://blog.dscpl.com.au/2012/10/obligations-for-calling-close-on.html
if iterable and hasattr(iterable, 'close') and callable(iterable.close):
try:
iterable.close()
except Exception:
self.handle_exception(environ)
def handle_slow_request(self, environ, dt):
headers = dict(get_headers(environ))
if 'Authorization' in headers:
headers['Authorization'] = 'redacted'
if 'Cookie' in headers:
headers['Cookie'] = 'redacted'
cak = environ.get('controller_action_key', None) or environ.get('PATH_INFO', "NOPATH").strip('/').replace('/', '.')
event_id = self.client.captureMessage(
f"SLOREQ: {cak}",
data={
'sentry.interfaces.Http': {
'method': environ.get('REQUEST_METHOD'),
'url': get_current_url(environ, strip_querystring=True),
'query_string': environ.get('QUERY_STRING'),
'headers': headers,
'env': dict(get_environ(environ)),
}
},
extra={
'request_id': environ.get('request_id', 'Unknown'),
'request_duration_millis': dt * 1000
},
level="warning",
tags={
'type': 'sloreq',
'action_key': cak
}
)
# Galaxy: store event_id in environment so we can show it to the user
environ['sentry_event_id'] = event_id
return event_id
def handle_exception(self, environ):
headers = dict(get_headers(environ))
# Authorization header for REMOTE_USER sites consists of a base64() of
# their plaintext password. It is a security issue for this password to
# be exposed to a third party system which may or may not be under
# control of the same administrators as the local Authentication
# system. E.g. university LDAP systems.
if 'Authorization' in headers:
# Redact so the administrator knows that a value is indeed present.
headers['Authorization'] = 'redacted'
# Passing cookies allows for impersonation of users (depending on
# remote service) and can be considered a security risk as well. For
# multiple services running alongside Galaxy on the same host, this
# could allow a sentry user with access to logs to impersonate a user
# on another service. In the case of services like Jupyter, this can be
# a serious concern as that would allow for terminal access. Furthermore,
# very little debugging information can be gained as a result of having
# access to all of the users cookies (including Galaxy cookies)
if 'Cookie' in headers:
headers['Cookie'] = 'redacted'
event_id = self.client.captureException(
data={
'sentry.interfaces.Http': {
'method': environ.get('REQUEST_METHOD'),
'url': get_current_url(environ, strip_querystring=True),
'query_string': environ.get('QUERY_STRING'),
# TODO
# 'data': environ.get('wsgi.input'),
'headers': headers,
'env': dict(get_environ(environ)),
}
},
# Galaxy: add request id from environment if available
extra={
'request_id': environ.get('request_id', 'Unknown')
}
)
# Galaxy: store event_id in environment so we can show it to the user
environ['sentry_event_id'] = event_id
return event_id
+2 -3
View File
@@ -1369,10 +1369,9 @@ def wrap_in_middleware(app, global_conf, application_stack, **local_conf):
# If sentry logging is enabled, log here before propogating up to
# the error middleware
sentry_dsn = conf.get('sentry_dsn', None)
sentry_sloreq = float(conf.get('sentry_sloreq_threshold', 0))
if sentry_dsn:
from galaxy.web.framework.middleware.sentry import Sentry
app = wrap_if_allowed(app, stack, Sentry, args=(sentry_dsn, sentry_sloreq))
from sentry_sdk.integrations.wsgi import SentryWsgiMiddleware
app = wrap_if_allowed(app, stack, SentryWsgiMiddleware)
# Various debug middleware that can only be turned on if the debug
# flag is set, either because they are insecure or greatly hurt
# performance
+5 -8
View File
@@ -2029,16 +2029,13 @@ mapping:
indicated sentry instance. This connection string is available in your
sentry instance under <project_name> -> Settings -> API Keys.
sentry_sloreq_threshold:
type: float
default: 0.0
sentry_event_level:
type: str
default: ERROR
required: false
desc: |
Sentry slow request logging. Requests slower than the threshold
indicated below will be sent as events to the configured Sentry
server (above, sentry_dsn). A value of '0' is disabled. For
example, you would set this to .005 to log all queries taking longer
than 5 milliseconds.
Determines the minimum log level that will be sent as an event to Sentry.
Possible values are DEBUG, INFO, WARNING, ERROR or CRITICAL.
statsd_host:
type: str
+5
View File
@@ -80,6 +80,11 @@ def add_galaxy_middleware(app: FastAPI, gx_app):
nginx_x_accel_redirect_base = gx_app.config.nginx_x_accel_redirect_base
apache_xsendfile = gx_app.config.apache_xsendfile
if gx_app.config.sentry_dsn:
from sentry_sdk.integrations.asgi import SentryAsgiMiddleware
app.add_middleware(SentryAsgiMiddleware)
if nginx_x_accel_redirect_base or apache_xsendfile:
@app.middleware("http")