diff --git a/changelog.d/10092.bugfix b/changelog.d/10092.bugfix
new file mode 100644
index 0000000000..09b2aba7ff
--- /dev/null
+++ b/changelog.d/10092.bugfix
@@ -0,0 +1 @@
+Fix a bug in the `force_tracing_for_users` option introduced in Synapse v1.35 which meant that the OpenTracing spans produced were missing most tags.
diff --git a/synapse/api/auth.py b/synapse/api/auth.py
index 458306eba5..26a3b38918 100644
--- a/synapse/api/auth.py
+++ b/synapse/api/auth.py
@@ -206,11 +206,11 @@ class Auth:
requester = create_requester(user_id, app_service=app_service)
request.requester = user_id
+ if user_id in self._force_tracing_for_users:
+ opentracing.set_tag(opentracing.tags.SAMPLING_PRIORITY, 1)
opentracing.set_tag("authenticated_entity", user_id)
opentracing.set_tag("user_id", user_id)
opentracing.set_tag("appservice_id", app_service.id)
- if user_id in self._force_tracing_for_users:
- opentracing.set_tag(opentracing.tags.SAMPLING_PRIORITY, 1)
return requester
@@ -259,12 +259,12 @@ class Auth:
)
request.requester = requester
+ if user_info.token_owner in self._force_tracing_for_users:
+ opentracing.set_tag(opentracing.tags.SAMPLING_PRIORITY, 1)
opentracing.set_tag("authenticated_entity", user_info.token_owner)
opentracing.set_tag("user_id", user_info.user_id)
if device_id:
opentracing.set_tag("device_id", device_id)
- if user_info.token_owner in self._force_tracing_for_users:
- opentracing.set_tag(opentracing.tags.SAMPLING_PRIORITY, 1)
return requester
except KeyError:
diff --git a/synapse/federation/transport/server.py b/synapse/federation/transport/server.py
index 59e0a434dc..fdeaa0f37c 100644
--- a/synapse/federation/transport/server.py
+++ b/synapse/federation/transport/server.py
@@ -37,6 +37,7 @@ from synapse.http.servlet import (
)
from synapse.logging.context import run_in_background
from synapse.logging.opentracing import (
+ SynapseTags,
start_active_span,
start_active_span_from_request,
tags,
@@ -314,7 +315,7 @@ class BaseFederationServlet:
raise
request_tags = {
- "request_id": request.get_request_id(),
+ SynapseTags.REQUEST_ID: request.get_request_id(),
tags.SPAN_KIND: tags.SPAN_KIND_RPC_SERVER,
tags.HTTP_METHOD: request.get_method(),
tags.HTTP_URL: request.get_redacted_uri(),
diff --git a/synapse/logging/opentracing.py b/synapse/logging/opentracing.py
index fba2fa3904..428831dad6 100644
--- a/synapse/logging/opentracing.py
+++ b/synapse/logging/opentracing.py
@@ -265,6 +265,12 @@ class SynapseTags:
# Whether the sync response has new data to be returned to the client.
SYNC_RESULT = "sync.new_data"
+ # incoming HTTP request ID (as written in the logs)
+ REQUEST_ID = "request_id"
+
+ # HTTP request tag (used to distinguish full vs incremental syncs, etc)
+ REQUEST_TAG = "request_tag"
+
# Block everything by default
# A regex which matches the server_names to expose traces for.
@@ -824,7 +830,7 @@ def trace_servlet(request: "SynapseRequest", extract_context: bool = False):
return
request_tags = {
- "request_id": request.get_request_id(),
+ SynapseTags.REQUEST_ID: request.get_request_id(),
tags.SPAN_KIND: tags.SPAN_KIND_RPC_SERVER,
tags.HTTP_METHOD: request.get_method(),
tags.HTTP_URL: request.get_redacted_uri(),
@@ -833,9 +839,9 @@ def trace_servlet(request: "SynapseRequest", extract_context: bool = False):
request_name = request.request_metrics.name
if extract_context:
- scope = start_active_span_from_request(request, request_name, tags=request_tags)
+ scope = start_active_span_from_request(request, request_name)
else:
- scope = start_active_span(request_name, tags=request_tags)
+ scope = start_active_span(request_name)
with scope:
try:
@@ -845,4 +851,11 @@ def trace_servlet(request: "SynapseRequest", extract_context: bool = False):
# with JsonResource).
scope.span.set_operation_name(request.request_metrics.name)
- scope.span.set_tag("request_tag", request.request_metrics.start_context.tag)
+ # set the tags *after* the servlet completes, in case it decided to
+ # prioritise the span (tags will get dropped on unprioritised spans)
+ request_tags[
+ SynapseTags.REQUEST_TAG
+ ] = request.request_metrics.start_context.tag
+
+ for k, v in request_tags.items():
+ scope.span.set_tag(k, v)
diff --git a/synapse/metrics/background_process_metrics.py b/synapse/metrics/background_process_metrics.py
index 714caf84c3..0d6d643d35 100644
--- a/synapse/metrics/background_process_metrics.py
+++ b/synapse/metrics/background_process_metrics.py
@@ -22,7 +22,11 @@ from prometheus_client.core import REGISTRY, Counter, Gauge
from twisted.internet import defer
from synapse.logging.context import LoggingContext, PreserveLoggingContext
-from synapse.logging.opentracing import noop_context_manager, start_active_span
+from synapse.logging.opentracing import (
+ SynapseTags,
+ noop_context_manager,
+ start_active_span,
+)
from synapse.util.async_helpers import maybe_awaitable
if TYPE_CHECKING:
@@ -202,7 +206,9 @@ def run_as_background_process(desc: str, func, *args, bg_start_span=True, **kwar
try:
ctx = noop_context_manager()
if bg_start_span:
- ctx = start_active_span(desc, tags={"request_id": str(context)})
+ ctx = start_active_span(
+ desc, tags={SynapseTags.REQUEST_ID: str(context)}
+ )
with ctx:
return await maybe_awaitable(func(*args, **kwargs))
except Exception:
|
