From 39aec640e34b6a80c8abd94150ba211de86772bc Mon Sep 17 00:00:00 2001 From: Eric Eastwood Date: Tue, 6 Sep 2022 16:47:35 -0500 Subject: [PATCH 1/5] Stop splitting traces across many services (by instance name) --- synapse/logging/opentracing.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/synapse/logging/opentracing.py b/synapse/logging/opentracing.py index 482316a1ff90..2c935c0ae178 100644 --- a/synapse/logging/opentracing.py +++ b/synapse/logging/opentracing.py @@ -443,7 +443,7 @@ def init_tracer(hs: "HomeServer") -> None: config = JaegerConfig( config=hs.config.tracing.jaeger_config, - service_name=f"{hs.config.server.server_name} {hs.get_instance_name()}", + service_name=f"{hs.config.server.server_name} - Synapse", scope_manager=LogContextScopeManager(), metrics_factory=PrometheusMetricsFactory(), ) From 0f023f307dd75f791363ad787c30460565dbe121 Mon Sep 17 00:00:00 2001 From: Eric Eastwood Date: Tue, 6 Sep 2022 17:19:57 -0500 Subject: [PATCH 2/5] Add changelog --- changelog.d/13729.misc | 1 + 1 file changed, 1 insertion(+) create mode 100644 changelog.d/13729.misc diff --git a/changelog.d/13729.misc b/changelog.d/13729.misc new file mode 100644 index 000000000000..538bac0b14ef --- /dev/null +++ b/changelog.d/13729.misc @@ -0,0 +1 @@ +Collect traces under the same service name instead of splitting across many services by instance name. From 341e7c91340b682f29b5b1d03bb85ad9a46c6f50 Mon Sep 17 00:00:00 2001 From: Eric Eastwood Date: Thu, 8 Sep 2022 13:49:56 -0500 Subject: [PATCH 3/5] Strip number suffix from instance name to consolidate services that traces are spread over See https://github.com/matrix-org/synapse/pull/13729#discussion_r966070287 --- synapse/logging/opentracing.py | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/synapse/logging/opentracing.py b/synapse/logging/opentracing.py index 2c935c0ae178..58ab17509a95 100644 --- a/synapse/logging/opentracing.py +++ b/synapse/logging/opentracing.py @@ -203,6 +203,9 @@ def set_fates(clotho, lachesis, atropos, father="Zues", mother="Themis"): # Helper class +# Matches the number suffix in an instance name like "matrix.org client_reader-8" +STRIP_INSTANCE_NUMBER_SUFFIX_REGEX = re.compile(r"[_-]?\d+$") + class _DummyTagNames: """wrapper of opentracings tags. We need to have them if we @@ -443,7 +446,9 @@ def init_tracer(hs: "HomeServer") -> None: config = JaegerConfig( config=hs.config.tracing.jaeger_config, - service_name=f"{hs.config.server.server_name} - Synapse", + service_name=re.sub( + STRIP_INSTANCE_NUMBER_SUFFIX_REGEX, "", hs.config.server.server_name + ), scope_manager=LogContextScopeManager(), metrics_factory=PrometheusMetricsFactory(), ) From 3418e86f2fb8019ca5c346665e0db6981f56faa8 Mon Sep 17 00:00:00 2001 From: Eric Eastwood Date: Thu, 8 Sep 2022 13:51:33 -0500 Subject: [PATCH 4/5] Update changelog --- changelog.d/13729.misc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/changelog.d/13729.misc b/changelog.d/13729.misc index 538bac0b14ef..c6a6f617e337 100644 --- a/changelog.d/13729.misc +++ b/changelog.d/13729.misc @@ -1 +1 @@ -Collect traces under the same service name instead of splitting across many services by instance name. +Strip number suffix from instance name to consolidate services that traces are spread over. From 8b5579b840465268d60715b5590f5632219395ca Mon Sep 17 00:00:00 2001 From: Eric Eastwood Date: Thu, 8 Sep 2022 14:03:23 -0500 Subject: [PATCH 5/5] Actually strip the right thing --- synapse/logging/opentracing.py | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/synapse/logging/opentracing.py b/synapse/logging/opentracing.py index 58ab17509a95..adf3f54770d8 100644 --- a/synapse/logging/opentracing.py +++ b/synapse/logging/opentracing.py @@ -444,11 +444,17 @@ def init_tracer(hs: "HomeServer") -> None: from jaeger_client.metrics.prometheus import PrometheusMetricsFactory + # Instance names are opaque strings but by stripping off the number suffix, + # we can get something that looks like a "worker type", e.g. + # "client_reader-1" -> "client_reader" so we don't spread the traces across + # so many services. + instance_name_by_type = re.sub( + STRIP_INSTANCE_NUMBER_SUFFIX_REGEX, "", hs.get_instance_name() + ) + config = JaegerConfig( config=hs.config.tracing.jaeger_config, - service_name=re.sub( - STRIP_INSTANCE_NUMBER_SUFFIX_REGEX, "", hs.config.server.server_name - ), + service_name=f"{hs.config.server.server_name} {instance_name_by_type}", scope_manager=LogContextScopeManager(), metrics_factory=PrometheusMetricsFactory(), )