From a31eb0489c63b23abecac313473329f98a95930e Mon Sep 17 00:00:00 2001 From: mvdbeek Date: Fri, 10 Feb 2023 12:52:38 +0100 Subject: [PATCH 1/2] Use sentry-fastapi integration, expose sampling rate --- doc/source/admin/galaxy_options.rst | 18 ++++++++++++++++-- lib/galaxy/app.py | 1 + lib/galaxy/config/sample/galaxy.yml.sample | 5 +++++ lib/galaxy/config/sample/tool_shed.yml.sample | 5 +++++ lib/galaxy/config/schemas/config_schema.yml | 9 +++++++++ .../config/schemas/tool_shed_config_schema.yml | 9 +++++++++ .../dependencies/conditional-requirements.txt | 2 +- lib/galaxy/webapps/galaxy/buildapp.py | 7 +------ lib/galaxy/webapps/galaxy/fast_app.py | 5 ----- lib/tool_shed/webapp/buildapp.py | 7 ------- 10 files changed, 47 insertions(+), 21 deletions(-) diff --git a/doc/source/admin/galaxy_options.rst b/doc/source/admin/galaxy_options.rst index f36e3162429..b6fa3a4ed94 100644 --- a/doc/source/admin/galaxy_options.rst +++ b/doc/source/admin/galaxy_options.rst @@ -2914,6 +2914,19 @@ :Type: str +~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ +``sentry_traces_sample_rate`` +~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ + +:Description: + Set to a number between 0 and 1. With this option set, every + transaction created will have that percentage chance of being sent + to Sentry. A value higher than 0 is required to analyze + performance. +:Default: ``0.0`` +:Type: float + + ~~~~~~~~~~~~~~~ ``statsd_host`` ~~~~~~~~~~~~~~~ @@ -5076,14 +5089,15 @@ :Type: str - ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ ``enable_beacon_integration`` ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ :Description: - Enables user preferences and api endpoint for the beacon integration. + Enables user preferences and api endpoint for the beacon + integration. :Default: ``false`` :Type: bool + diff --git a/lib/galaxy/app.py b/lib/galaxy/app.py index a4b50420185..e2d61248aa8 100644 --- a/lib/galaxy/app.py +++ b/lib/galaxy/app.py @@ -202,6 +202,7 @@ class SentryClientMixin: self.config.sentry_dsn, release=f"{self.config.version_major}.{self.config.version_minor}", integrations=[sentry_logging], + traces_sample_rate=self.config.sentry_traces_sample_rate, ) self.application_stack.register_postfork_function(postfork_sentry_client) diff --git a/lib/galaxy/config/sample/galaxy.yml.sample b/lib/galaxy/config/sample/galaxy.yml.sample index 21b7e587206..d4579e56adb 100644 --- a/lib/galaxy/config/sample/galaxy.yml.sample +++ b/lib/galaxy/config/sample/galaxy.yml.sample @@ -1667,6 +1667,11 @@ galaxy: # Sentry. Possible values are DEBUG, INFO, WARNING, ERROR or CRITICAL. #sentry_event_level: ERROR + # Set to a number between 0 and 1. With this option set, every + # transaction created will have that percentage chance of being sent + # to Sentry. A value higher than 0 is required to analyze performance. + #sentry_traces_sample_rate: 0.0 + # Log to statsd Statsd is an external statistics aggregator # (https://github.com/etsy/statsd) Enabling the following options will # cause galaxy to log request timing and other statistics to the diff --git a/lib/galaxy/config/sample/tool_shed.yml.sample b/lib/galaxy/config/sample/tool_shed.yml.sample index fe8883a5a80..6be4f308e56 100644 --- a/lib/galaxy/config/sample/tool_shed.yml.sample +++ b/lib/galaxy/config/sample/tool_shed.yml.sample @@ -311,6 +311,11 @@ tool_shed: # Sentry. Possible values are DEBUG, INFO, WARNING, ERROR or CRITICAL. #sentry_event_level: ERROR + # Set to a number between 0 and 1. With this option set, every + # transaction created will have that percentage chance of being sent + # to Sentry. A value higher than 0 is required to analyze performance. + #sentry_traces_sample_rate: 0.0 + # Galaxy Session Timeout This provides a timeout (in minutes) after # which a user will have to log back in. A duration of 0 disables this # feature. diff --git a/lib/galaxy/config/schemas/config_schema.yml b/lib/galaxy/config/schemas/config_schema.yml index 11a58011dec..f0c16d6ebc6 100644 --- a/lib/galaxy/config/schemas/config_schema.yml +++ b/lib/galaxy/config/schemas/config_schema.yml @@ -2108,6 +2108,15 @@ mapping: Determines the minimum log level that will be sent as an event to Sentry. Possible values are DEBUG, INFO, WARNING, ERROR or CRITICAL. + sentry_traces_sample_rate: + type: float + default: 0.0 + required: false + desc: | + Set to a number between 0 and 1. With this option set, every transaction created + will have that percentage chance of being sent to Sentry. A value higher than 0 + is required to analyze performance. + statsd_host: type: str required: false diff --git a/lib/galaxy/config/schemas/tool_shed_config_schema.yml b/lib/galaxy/config/schemas/tool_shed_config_schema.yml index 39b5079221a..7b2f5bcfc58 100644 --- a/lib/galaxy/config/schemas/tool_shed_config_schema.yml +++ b/lib/galaxy/config/schemas/tool_shed_config_schema.yml @@ -557,6 +557,15 @@ mapping: Determines the minimum log level that will be sent as an event to Sentry. Possible values are DEBUG, INFO, WARNING, ERROR or CRITICAL. + sentry_traces_sample_rate: + type: float + default: 0.0 + required: false + desc: | + Set to a number between 0 and 1. With this option set, every transaction created + will have that percentage chance of being sent to Sentry. A value higher than 0 + is required to analyze performance. + session_duration: type: int default: 0 diff --git a/lib/galaxy/dependencies/conditional-requirements.txt b/lib/galaxy/dependencies/conditional-requirements.txt index 0c2e60d96b0..24a076a389a 100644 --- a/lib/galaxy/dependencies/conditional-requirements.txt +++ b/lib/galaxy/dependencies/conditional-requirements.txt @@ -2,7 +2,7 @@ psycopg2-binary==2.9.5 mysqlclient fluent-logger -sentry-sdk +sentry-sdk[fastapi] pbs_python drmaa statsd diff --git a/lib/galaxy/webapps/galaxy/buildapp.py b/lib/galaxy/webapps/galaxy/buildapp.py index 46ca9c58779..a9bcaba1131 100644 --- a/lib/galaxy/webapps/galaxy/buildapp.py +++ b/lib/galaxy/webapps/galaxy/buildapp.py @@ -66,6 +66,7 @@ def app_pair(global_conf, load_app_kwds=None, wsgi_preflight=True, **kwargs): # Call app's shutdown method when the interpeter exits, this cleanly stops # the various Galaxy application daemon threads app.application_stack.register_postfork_function(atexit.register, app.shutdown) + # Create the universe WSGI application webapp = GalaxyWebApplication(app, session_cookie="galaxysession", name="galaxy") @@ -1304,13 +1305,7 @@ def wrap_in_middleware(app, global_conf, application_stack, **local_conf): from paste import recursive app = wrap_if_allowed(app, stack, recursive.RecursiveMiddleware, args=(conf,)) - # If sentry logging is enabled, log here before propogating up to - # the error middleware - sentry_dsn = conf.get("sentry_dsn", None) - if sentry_dsn: - from sentry_sdk.integrations.wsgi import SentryWsgiMiddleware - app = wrap_if_allowed(app, stack, SentryWsgiMiddleware) # Error middleware app = wrap_if_allowed(app, stack, ErrorMiddleware, args=(conf,)) # Transaction logging (apache access.log style) diff --git a/lib/galaxy/webapps/galaxy/fast_app.py b/lib/galaxy/webapps/galaxy/fast_app.py index be97aa4beaf..49ccdb71868 100644 --- a/lib/galaxy/webapps/galaxy/fast_app.py +++ b/lib/galaxy/webapps/galaxy/fast_app.py @@ -107,11 +107,6 @@ def add_galaxy_middleware(app: FastAPI, gx_app): GalaxyFileResponse.nginx_x_accel_redirect_base = gx_app.config.nginx_x_accel_redirect_base GalaxyFileResponse.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 gx_app.config.get("allowed_origin_hostnames", None): app.add_middleware( GalaxyCORSMiddleware, diff --git a/lib/tool_shed/webapp/buildapp.py b/lib/tool_shed/webapp/buildapp.py index c69599a3da3..3e754af348d 100644 --- a/lib/tool_shed/webapp/buildapp.py +++ b/lib/tool_shed/webapp/buildapp.py @@ -267,14 +267,7 @@ def wrap_in_middleware(app, global_conf, application_stack, **local_conf): from paste.translogger import TransLogger app = wrap_if_allowed(app, stack, TransLogger) - # If sentry logging is enabled, log here before propogating up to - # the error middleware - # TODO sentry config is duplicated between tool_shed/galaxy, refactor this. - sentry_dsn = conf.get("sentry_dsn", None) - if sentry_dsn: - from sentry_sdk.integrations.wsgi import SentryWsgiMiddleware - app = wrap_if_allowed(app, stack, SentryWsgiMiddleware) # X-Forwarded-Host handling from galaxy.web.framework.middleware.xforwardedhost import XForwardedHostMiddleware From 29d7d14ae1d20d91808f9a5cbe7e23e065dd500b Mon Sep 17 00:00:00 2001 From: mvdbeek Date: Fri, 10 Feb 2023 18:36:57 +0100 Subject: [PATCH 2/2] Update FastAPI --- lib/galaxy/dependencies/pinned-requirements.txt | 4 ++-- .../unit/webapps/test_request_scoped_sqlalchemy_sessions.py | 6 +----- 2 files changed, 3 insertions(+), 7 deletions(-) diff --git a/lib/galaxy/dependencies/pinned-requirements.txt b/lib/galaxy/dependencies/pinned-requirements.txt index 1bc3767683b..9045743d850 100644 --- a/lib/galaxy/dependencies/pinned-requirements.txt +++ b/lib/galaxy/dependencies/pinned-requirements.txt @@ -61,7 +61,7 @@ ecdsa==0.18.0 ; python_version >= "3.7" and python_version < "3.12" edam-ontology==1.25.2 ; python_version >= "3.7" and python_version < "3.12" email-validator==1.3.0 ; python_version >= "3.7" and python_version < "3.12" fastapi-utils==0.2.1 ; python_version >= "3.7" and python_version < "3.12" -fastapi==0.89.1 ; python_version >= "3.7" and python_version < "3.12" +fastapi==0.91.0 ; python_version >= "3.7" and python_version < "3.12" flupy==1.2.0 ; python_version >= "3.7" and python_version < "3.12" frozenlist==1.3.3 ; python_version >= "3.7" and python_version < "3.12" fs==2.4.16 ; python_version >= "3.7" and python_version < "3.12" @@ -171,7 +171,7 @@ sqlalchemy==1.4.46 ; python_version >= "3.7" and python_version < "3.12" sqlitedict==2.1.0 ; python_version >= "3.7" and python_version < "3.12" sqlparse==0.4.3 ; python_version >= "3.7" and python_version < "3.12" starlette-context==0.3.5 ; python_version >= "3.7" and python_version < "3.12" -starlette==0.22.0 ; python_version >= "3.7" and python_version < "3.12" +starlette==0.24.0 ; python_version >= "3.7" and python_version < "3.12" supervisor==4.2.5 ; python_version >= "3.7" and python_version < "3.12" svgwrite==1.4.3 ; python_version >= "3.7" and python_version < "3.12" tempita==0.5.2 ; python_version >= "3.7" and python_version < "3.12" diff --git a/test/unit/webapps/test_request_scoped_sqlalchemy_sessions.py b/test/unit/webapps/test_request_scoped_sqlalchemy_sessions.py index 836662467b5..31d90e28a12 100644 --- a/test/unit/webapps/test_request_scoped_sqlalchemy_sessions.py +++ b/test/unit/webapps/test_request_scoped_sqlalchemy_sessions.py @@ -15,6 +15,7 @@ from galaxy.app_unittest_utils.galaxy_mock import MockApp from galaxy.webapps.base.api import add_request_id_middleware app = FastAPI() +add_request_id_middleware(app) GX_APP = None @@ -95,7 +96,6 @@ def assert_scoped_session_is_thread_local(gx_app): @pytest.mark.asyncio async def test_request_scoped_sa_session_single_request(): - add_request_id_middleware(app) async with AsyncClient(app=app, base_url="http://test") as client: response = await client.get("/") assert response.status_code == 200 @@ -106,7 +106,6 @@ async def test_request_scoped_sa_session_single_request(): @pytest.mark.asyncio async def test_request_scoped_sa_session_exception(): - add_request_id_middleware(app) async with AsyncClient(app=app, base_url="http://test") as client: with pytest.raises(UnexpectedException): await client.get("/internal_server_error") @@ -116,7 +115,6 @@ async def test_request_scoped_sa_session_exception(): @pytest.mark.asyncio async def test_request_scoped_sa_session_concurrent_requests_sync(): - add_request_id_middleware(app) async with AsyncClient(app=app, base_url="http://test") as client: awaitables = (client.get("/sync_wait") for _ in range(10)) result = await asyncio.gather(*awaitables) @@ -131,7 +129,6 @@ async def test_request_scoped_sa_session_concurrent_requests_sync(): @pytest.mark.asyncio async def test_request_scoped_sa_session_concurrent_requests_async(): - add_request_id_middleware(app) async with AsyncClient(app=app, base_url="http://test") as client: awaitables = (client.get("/async_wait") for _ in range(10)) result = await asyncio.gather(*awaitables) @@ -146,7 +143,6 @@ async def test_request_scoped_sa_session_concurrent_requests_async(): @pytest.mark.asyncio async def test_request_scoped_sa_session_concurrent_requests_and_background_thread(): - add_request_id_middleware(app) loop = asyncio.get_running_loop() target = functools.partial(assert_scoped_session_is_thread_local, GX_APP) with concurrent.futures.ThreadPoolExecutor() as pool: