From ef2228c890b44963c65f086eb1246c27ef43d256 Mon Sep 17 00:00:00 2001 From: Erik Johnston Date: Tue, 12 Feb 2019 13:55:58 +0000 Subject: [PATCH 1/7] Basic sentry integration --- synapse/app/_base.py | 22 ++++++++++++++++++++++ synapse/config/metrics.py | 8 ++++++++ synapse/python_dependencies.py | 1 + 3 files changed, 31 insertions(+) diff --git a/synapse/app/_base.py b/synapse/app/_base.py index 5b0ca312e..d681ff524 100644 --- a/synapse/app/_base.py +++ b/synapse/app/_base.py @@ -25,10 +25,12 @@ from daemonize import Daemonize from twisted.internet import error, reactor from twisted.protocols.tls import TLSMemoryBIOFactory +import synapse from synapse.app import check_bind_error from synapse.crypto import context_factory from synapse.util import PreserveLoggingContext from synapse.util.rlimit import change_resource_limit +from synapse.util.versionstring import get_version_string logger = logging.getLogger(__name__) @@ -266,9 +268,29 @@ def start(hs, listeners=None): # It is now safe to start your Synapse. hs.start_listening(listeners) hs.get_datastore().start_profiling() + + setup_sentry_io(hs) except Exception: traceback.print_exc(file=sys.stderr) reactor = hs.get_reactor() if reactor.running: reactor.stop() sys.exit(1) + + +def setup_sentry_io(hs): + if not hs.config.sentry_enabled: + return + + import sentry_sdk + sentry_sdk.init( + dsn=hs.config.sentry_dsn, + release=get_version_string(synapse), + ) + with sentry_sdk.configure_scope() as scope: + scope.set_tag("matrix_server_name", hs.config.server_name) + + app = hs.config.worker_app if hs.config.worker_app else "synapse.app.homeserver" + name = hs.config.worker_name if hs.config.worker_name else "master" + scope.set_tag("worker_app", app) + scope.set_tag("worker_name", name) diff --git a/synapse/config/metrics.py b/synapse/config/metrics.py index 718c43ae0..f312010a9 100644 --- a/synapse/config/metrics.py +++ b/synapse/config/metrics.py @@ -23,12 +23,20 @@ class MetricsConfig(Config): self.metrics_port = config.get("metrics_port") self.metrics_bind_host = config.get("metrics_bind_host", "127.0.0.1") + self.sentry_enabled = "sentry" in config + if self.sentry_enabled: + self.sentry_dsn = config["sentry"]["dsn"] + def default_config(self, report_stats=None, **kwargs): res = """\ ## Metrics ### # Enable collection and rendering of performance metrics enable_metrics: False + + # Enable sentry.io integration + #sentry: + # dsn: "..." """ if report_stats is None: diff --git a/synapse/python_dependencies.py b/synapse/python_dependencies.py index 5d087ee26..444fc2a97 100644 --- a/synapse/python_dependencies.py +++ b/synapse/python_dependencies.py @@ -86,6 +86,7 @@ CONDITIONAL_REQUIREMENTS = { "saml2": ["pysaml2>=4.5.0"], "url_preview": ["lxml>=3.5.0"], "test": ["mock>=2.0", "parameterized"], + "sentry": ["sentry-sdk>=0.7.2"], } From 6a8f902edbb1b003ae4a5f1c78d9c09fa4e87dfc Mon Sep 17 00:00:00 2001 From: Erik Johnston Date: Tue, 12 Feb 2019 16:01:41 +0000 Subject: [PATCH 2/7] Raise an appropriate error message if sentry_sdk missing --- synapse/config/metrics.py | 16 +++++++++++++++- 1 file changed, 15 insertions(+), 1 deletion(-) diff --git a/synapse/config/metrics.py b/synapse/config/metrics.py index f312010a9..7bab8b0b2 100644 --- a/synapse/config/metrics.py +++ b/synapse/config/metrics.py @@ -13,7 +13,16 @@ # See the License for the specific language governing permissions and # limitations under the License. -from ._base import Config +from ._base import Config, ConfigError + +MISSING_SENTRY = ( + """Missing sentry_sdk library. This is required for enable sentry.io + integration. + + Install by running: + pip install sentry_sdk + """ +) class MetricsConfig(Config): @@ -25,6 +34,11 @@ class MetricsConfig(Config): self.sentry_enabled = "sentry" in config if self.sentry_enabled: + try: + import sentry_sdk # noqa F401 + except ImportError: + raise ConfigError(MISSING_SENTRY) + self.sentry_dsn = config["sentry"]["dsn"] def default_config(self, report_stats=None, **kwargs): From 93f7d2df3e8bb44e4f9fb6c5ce3fc23a86c30c1a Mon Sep 17 00:00:00 2001 From: Erik Johnston Date: Tue, 12 Feb 2019 16:03:40 +0000 Subject: [PATCH 3/7] Comments --- synapse/app/_base.py | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/synapse/app/_base.py b/synapse/app/_base.py index d681ff524..482f0e22e 100644 --- a/synapse/app/_base.py +++ b/synapse/app/_base.py @@ -279,6 +279,12 @@ def start(hs, listeners=None): def setup_sentry_io(hs): + """Enable sentry.io integration, if enabled in configuration + + Args: + hs (synapse.server.HomeServer) + """ + if not hs.config.sentry_enabled: return @@ -287,6 +293,8 @@ def setup_sentry_io(hs): dsn=hs.config.sentry_dsn, release=get_version_string(synapse), ) + + # We set some default tags that give some context to this instance with sentry_sdk.configure_scope() as scope: scope.set_tag("matrix_server_name", hs.config.server_name) From dc7078905600cc3945d3eb1dcf008dd7a196d24a Mon Sep 17 00:00:00 2001 From: Erik Johnston Date: Tue, 12 Feb 2019 16:07:43 +0000 Subject: [PATCH 4/7] Newsfile --- changelog.d/4632.feature | 1 + 1 file changed, 1 insertion(+) create mode 100644 changelog.d/4632.feature diff --git a/changelog.d/4632.feature b/changelog.d/4632.feature new file mode 100644 index 000000000..0bdeb3738 --- /dev/null +++ b/changelog.d/4632.feature @@ -0,0 +1 @@ +Add basic optional sentry.io integration From 6cb415b63fd58ab253f8519f725a581a0e15044e Mon Sep 17 00:00:00 2001 From: Erik Johnston Date: Wed, 13 Feb 2019 16:14:37 +0000 Subject: [PATCH 5/7] Fixup comments and add warning --- changelog.d/4632.feature | 2 +- synapse/app/_base.py | 6 +++--- synapse/config/metrics.py | 9 +++++++-- 3 files changed, 11 insertions(+), 6 deletions(-) diff --git a/changelog.d/4632.feature b/changelog.d/4632.feature index 0bdeb3738..d053ab5a2 100644 --- a/changelog.d/4632.feature +++ b/changelog.d/4632.feature @@ -1 +1 @@ -Add basic optional sentry.io integration +Add basic optional sentry integration diff --git a/synapse/app/_base.py b/synapse/app/_base.py index 482f0e22e..0284151d0 100644 --- a/synapse/app/_base.py +++ b/synapse/app/_base.py @@ -269,7 +269,7 @@ def start(hs, listeners=None): hs.start_listening(listeners) hs.get_datastore().start_profiling() - setup_sentry_io(hs) + setup_sentry(hs) except Exception: traceback.print_exc(file=sys.stderr) reactor = hs.get_reactor() @@ -278,8 +278,8 @@ def start(hs, listeners=None): sys.exit(1) -def setup_sentry_io(hs): - """Enable sentry.io integration, if enabled in configuration +def setup_sentry(hs): + """Enable sentry integration, if enabled in configuration Args: hs (synapse.server.HomeServer) diff --git a/synapse/config/metrics.py b/synapse/config/metrics.py index 7bab8b0b2..185b895a2 100644 --- a/synapse/config/metrics.py +++ b/synapse/config/metrics.py @@ -16,7 +16,7 @@ from ._base import Config, ConfigError MISSING_SENTRY = ( - """Missing sentry_sdk library. This is required for enable sentry.io + """Missing sentry_sdk library. This is required for enable sentry integration. Install by running: @@ -48,7 +48,12 @@ class MetricsConfig(Config): # Enable collection and rendering of performance metrics enable_metrics: False - # Enable sentry.io integration + # Enable sentry integration + # NOTE: While attempts are made to ensure that the logs don't contain + # any sensitive information, this cannot be guaranteed. By enabling + # this option the sentry server may therefore receive sensitive + # information, and it in turn may then diseminate sensitive information + # through insecure notification channels if so configured. #sentry: # dsn: "..." """ From dc5efc92a82dbf96a7d4d054b0c842b9788bea2a Mon Sep 17 00:00:00 2001 From: Erik Johnston Date: Mon, 18 Feb 2019 13:52:49 +0000 Subject: [PATCH 6/7] Fixup --- synapse/config/metrics.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/synapse/config/metrics.py b/synapse/config/metrics.py index 185b895a2..fc72e32d2 100644 --- a/synapse/config/metrics.py +++ b/synapse/config/metrics.py @@ -16,7 +16,7 @@ from ._base import Config, ConfigError MISSING_SENTRY = ( - """Missing sentry_sdk library. This is required for enable sentry + """Missing sentry_sdk library. This is required to enable sentry integration. Install by running: From d328a93b51d22039e361178a5fcf952d9735cd3f Mon Sep 17 00:00:00 2001 From: Erik Johnston Date: Mon, 18 Feb 2019 16:53:56 +0000 Subject: [PATCH 7/7] Fixup error handling and message --- synapse/config/metrics.py | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/synapse/config/metrics.py b/synapse/config/metrics.py index fc72e32d2..35f107476 100644 --- a/synapse/config/metrics.py +++ b/synapse/config/metrics.py @@ -16,11 +16,8 @@ from ._base import Config, ConfigError MISSING_SENTRY = ( - """Missing sentry_sdk library. This is required to enable sentry + """Missing sentry-sdk library. This is required to enable sentry integration. - - Install by running: - pip install sentry_sdk """ ) @@ -39,7 +36,11 @@ class MetricsConfig(Config): except ImportError: raise ConfigError(MISSING_SENTRY) - self.sentry_dsn = config["sentry"]["dsn"] + self.sentry_dsn = config["sentry"].get("dsn") + if not self.sentry_dsn: + raise ConfigError( + "sentry.dsn field is required when sentry integration is enabled", + ) def default_config(self, report_stats=None, **kwargs): res = """\