summary refs log tree commit diff
diff options
context:
space:
mode:
authorEric Eastwood <erice@element.io>2022-09-13 16:29:51 -0500
committerEric Eastwood <erice@element.io>2022-09-13 16:53:14 -0500
commitb77d49f4410ceb42880af2882e70b3b3164955cc (patch)
tree6db246badd6986552c812310780a2796e46ab391
parentMerge branch 'develop' into madlittlemods/11850-migrate-to-opentelemetry (diff)
downloadsynapse-b77d49f4410ceb42880af2882e70b3b3164955cc.tar.xz
Hopefully fix problem when OTEL not installed with non recording span
```
Traceback (most recent call last):
  File "/usr/local/lib/python3.9/site-packages/synapse/http/server.py", line 306, in _async_render_wrapper
    callback_return = await self._async_render(request)
  File "/usr/local/lib/python3.9/site-packages/synapse/http/server.py", line 512, in _async_render
    callback_return = await raw_callback_return
  File "/usr/local/lib/python3.9/site-packages/synapse/federation/transport/server/_base.py", line 357, in new_func
    remote_parent_span = create_non_recording_span()
  File "/usr/local/lib/python3.9/site-packages/synapse/logging/tracing.py", line 502, in create_non_recording_span
    return opentelemetry.trace.NonRecordingSpan(
AttributeError: 'NoneType' object has no attribute 'trace'
```
-rw-r--r--synapse/federation/transport/server/_base.py1
-rw-r--r--synapse/logging/tracing.py37
-rw-r--r--synapse/metrics/background_process_metrics.py2
3 files changed, 23 insertions, 17 deletions
diff --git a/synapse/federation/transport/server/_base.py b/synapse/federation/transport/server/_base.py
index 441ff8ce71..cba690e795 100644
--- a/synapse/federation/transport/server/_base.py
+++ b/synapse/federation/transport/server/_base.py
@@ -320,6 +320,7 @@ class BaseFederationServlet:
             if origin and whitelisted_homeserver(origin):
                 origin_context = context_from_request(request)
 
+            remote_parent_span = None
             if origin_context:
                 local_servlet_span = get_active_span()
                 # Create a span which uses the `origin_context` as a parent
diff --git a/synapse/logging/tracing.py b/synapse/logging/tracing.py
index 2fe9875398..b92c1d96e3 100644
--- a/synapse/logging/tracing.py
+++ b/synapse/logging/tracing.py
@@ -487,17 +487,19 @@ def whitelisted_homeserver(destination: str) -> bool:
 
 
 def use_span(
-    span: "opentelemetry.trace.Span",
+    span: Optional["opentelemetry.trace.Span"],
     end_on_exit: bool = True,
-) -> ContextManager["opentelemetry.trace.Span"]:
-    if opentelemetry is None:
-        return contextlib.nullcontext()  # type: ignore[unreachable]
+) -> ContextManager[Optional["opentelemetry.trace.Span"]]:
+    if opentelemetry is None or span is None:
+        return contextlib.nullcontext()
 
     return opentelemetry.trace.use_span(span=span, end_on_exit=end_on_exit)
 
 
-def create_non_recording_span() -> "opentelemetry.trace.Span":
+def create_non_recording_span() -> Optional["opentelemetry.trace.Span"]:
     """Create a no-op span that does not record or become part of a recorded trace"""
+    if opentelemetry is None:
+        return None  # type: ignore[unreachable]
 
     return opentelemetry.trace.NonRecordingSpan(
         opentelemetry.trace.INVALID_SPAN_CONTEXT
@@ -554,7 +556,7 @@ def start_active_span(
     end_on_exit: bool = True,
     # For testing only
     tracer: Optional["opentelemetry.trace.Tracer"] = None,
-) -> ContextManager["opentelemetry.trace.Span"]:
+) -> ContextManager[Optional["opentelemetry.trace.Span"]]:
     if opentelemetry is None:
         return contextlib.nullcontext()  # type: ignore[unreachable]
 
@@ -588,7 +590,7 @@ def start_active_span_from_edu(
     operation_name: str,
     *,
     edu_content: Dict[str, Any],
-) -> ContextManager["opentelemetry.trace.Span"]:
+) -> ContextManager[Optional["opentelemetry.trace.Span"]]:
     """
     Extracts a span context from an edu and uses it to start a new active span
 
@@ -1003,17 +1005,20 @@ def trace_servlet(
         # finished sending the response to the client.
         end_on_exit=False,
     ) as span:
-        request.set_tracing_span(span)
+        if span:
+            request.set_tracing_span(span)
 
         inject_trace_id_into_response_headers(request.responseHeaders)
         try:
             yield
         finally:
-            # We set the operation name again in case its changed (which happens
-            # with JsonResource).
-            span.update_name(request.request_metrics.name)
-
-            if request.request_metrics.start_context.tag is not None:
-                span.set_attribute(
-                    SynapseTags.REQUEST_TAG, request.request_metrics.start_context.tag
-                )
+            if span:
+                # We set the operation name again in case its changed (which happens
+                # with JsonResource).
+                span.update_name(request.request_metrics.name)
+
+                if request.request_metrics.start_context.tag is not None:
+                    span.set_attribute(
+                        SynapseTags.REQUEST_TAG,
+                        request.request_metrics.start_context.tag,
+                    )
diff --git a/synapse/metrics/background_process_metrics.py b/synapse/metrics/background_process_metrics.py
index 59d956dd9d..ad639fe223 100644
--- a/synapse/metrics/background_process_metrics.py
+++ b/synapse/metrics/background_process_metrics.py
@@ -236,7 +236,7 @@ def run_as_background_process(
                         attributes={SynapseTags.REQUEST_ID: str(context)},
                     )
                 else:
-                    ctx = nullcontext()  # type: ignore[assignment]
+                    ctx = nullcontext()
                 with ctx:
                     return await func(*args, **kwargs)
             except Exception: