Skip to content
This repository has been archived by the owner on Apr 26, 2024. It is now read-only.

More type hints for synapse.logging #13103

Merged
merged 6 commits into from
Jun 30, 2022
Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions changelog.d/13103.misc
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Add missing type hints to `synapse.logging`.
3 changes: 0 additions & 3 deletions mypy.ini
Original file line number Diff line number Diff line change
Expand Up @@ -89,9 +89,6 @@ disallow_untyped_defs = False
[mypy-synapse.logging.opentracing]
disallow_untyped_defs = False

[mypy-synapse.logging.scopecontextmanager]
disallow_untyped_defs = False

[mypy-synapse.metrics._reactor_metrics]
disallow_untyped_defs = False
# This module imports select.epoll. That exists on Linux, but doesn't on macOS.
Expand Down
50 changes: 26 additions & 24 deletions synapse/logging/opentracing.py
Original file line number Diff line number Diff line change
Expand Up @@ -268,7 +268,7 @@ class _WrappedRustReporter(BaseReporter):

_reporter: Reporter = attr.Factory(Reporter)

def set_process(self, *args, **kwargs):
def set_process(self, *args: Any, **kwargs: Any) -> None:
return self._reporter.set_process(*args, **kwargs)

def report_span(self, span: "opentracing.Span") -> None:
Expand Down Expand Up @@ -339,12 +339,12 @@ def _only_if_tracing_inner(*args: P.args, **kwargs: P.kwargs) -> Optional[R]:
return _only_if_tracing_inner


def ensure_active_span(message, ret=None):
def ensure_active_span(message: str, ret=None):
"""Executes the operation only if opentracing is enabled and there is an active span.
If there is no active span it logs message at the error level.

Args:
message (str): Message which fills in "There was no active span when trying to %s"
message: Message which fills in "There was no active span when trying to %s"
in the error log if there is no active span and opentracing is enabled.
ret (object): return value if opentracing is None or there is no active span.

Expand Down Expand Up @@ -402,7 +402,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()}",
scope_manager=LogContextScopeManager(hs.config),
scope_manager=LogContextScopeManager(),
metrics_factory=PrometheusMetricsFactory(),
)

Expand Down Expand Up @@ -451,15 +451,15 @@ def whitelisted_homeserver(destination: str) -> bool:

# Could use kwargs but I want these to be explicit
def start_active_span(
operation_name,
child_of=None,
references=None,
tags=None,
start_time=None,
ignore_active_span=False,
finish_on_close=True,
operation_name: str,
child_of: Optional[Union["opentracing.Span", "opentracing.SpanContext"]] = None,
references: Optional[List["opentracing.Reference"]] = None,
tags: Optional[Dict[str, str]] = None,
start_time: Optional[float] = None,
ignore_active_span: bool = False,
finish_on_close: bool = True,
*,
tracer=None,
tracer: Optional["opentracing.Tracer"] = None,
):
"""Starts an active opentracing span.

Expand Down Expand Up @@ -493,11 +493,11 @@ def start_active_span(
def start_active_span_follows_from(
operation_name: str,
contexts: Collection,
child_of=None,
child_of: Optional[Union["opentracing.Span", "opentracing.SpanContext"]] = None,
start_time: Optional[float] = None,
*,
inherit_force_tracing=False,
tracer=None,
inherit_force_tracing: bool = False,
tracer: Optional["opentracing.Tracer"] = None,
):
"""Starts an active opentracing span, with additional references to previous spans

Expand Down Expand Up @@ -540,7 +540,7 @@ def start_active_span_from_edu(
edu_content: Dict[str, Any],
operation_name: str,
references: Optional[List["opentracing.Reference"]] = None,
tags: Optional[Dict] = None,
tags: Optional[Dict[str, str]] = None,
start_time: Optional[float] = None,
ignore_active_span: bool = False,
finish_on_close: bool = True,
Expand Down Expand Up @@ -617,23 +617,25 @@ def set_operation_name(operation_name: str) -> None:


@only_if_tracing
def force_tracing(span=Sentinel) -> None:
def force_tracing(span: "opentracing.Span" = Sentinel) -> None: # type: ignore[assignment]
clokep marked this conversation as resolved.
Show resolved Hide resolved
"""Force sampling for the active/given span and its children.

Args:
span: span to force tracing for. By default, the active span.
"""
if span is Sentinel:
span = opentracing.tracer.active_span
if span is None:
span_to_trace = opentracing.tracer.active_span
else:
span_to_trace = span
if span_to_trace is None:
clokep marked this conversation as resolved.
Show resolved Hide resolved
logger.error("No active span in force_tracing")
return

span.set_tag(opentracing.tags.SAMPLING_PRIORITY, 1)
span_to_trace.set_tag(opentracing.tags.SAMPLING_PRIORITY, 1)

# also set a bit of baggage, so that we have a way of figuring out if
# it is enabled later
span.set_baggage_item(SynapseBaggage.FORCE_TRACING, "1")
span_to_trace.set_baggage_item(SynapseBaggage.FORCE_TRACING, "1")


def is_context_forced_tracing(
Expand Down Expand Up @@ -789,7 +791,7 @@ def extract_text_map(carrier: Dict[str, str]) -> Optional["opentracing.SpanConte
# Tracing decorators


def trace(func=None, opname=None):
def trace(func=None, opname: Optional[str] = None):
"""
Decorator to trace a function.
Sets the operation name to that of the function's or that given
Expand Down Expand Up @@ -822,11 +824,11 @@ def _trace_inner(*args, **kwargs):
result = func(*args, **kwargs)
if isinstance(result, defer.Deferred):

def call_back(result):
def call_back(result: R) -> R:
scope.__exit__(None, None, None)
return result

def err_back(result):
def err_back(result: R) -> R:
scope.__exit__(None, None, None)
return result

Expand Down
35 changes: 19 additions & 16 deletions synapse/logging/scopecontextmanager.py
Original file line number Diff line number Diff line change
Expand Up @@ -16,11 +16,15 @@
from types import TracebackType
from typing import Optional, Type

from opentracing import Scope, ScopeManager
from opentracing import Scope, ScopeManager, Span

import twisted

from synapse.logging.context import current_context, nested_logging_context
from synapse.logging.context import (
LoggingContext,
current_context,
nested_logging_context,
)

logger = logging.getLogger(__name__)

Expand All @@ -35,11 +39,11 @@ class LogContextScopeManager(ScopeManager):
but currently that doesn't work due to https://twistedmatrix.com/trac/ticket/10301.
"""

def __init__(self, config):
def __init__(self) -> None:
pass
Comment on lines +42 to 43
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I didn't remove this because we're still overriding the default ScopeManager init method.


@property
def active(self):
def active(self) -> Optional[Scope]:
"""
Returns the currently active Scope which can be used to access the
currently active Scope.span.
Expand All @@ -48,19 +52,18 @@ def active(self):
Tracer.start_active_span() time.

Return:
(Scope) : the Scope that is active, or None if not
available.
The Scope that is active, or None if not available.
"""
ctx = current_context()
return ctx.scope

def activate(self, span, finish_on_close):
def activate(self, span: Span, finish_on_close: bool) -> Scope:
"""
Makes a Span active.
Args
span (Span): the span that should become active.
finish_on_close (Boolean): whether Span should be automatically
finished when Scope.close() is called.
span: the span that should become active.
finish_on_close: whether Span should be automatically finished when
Scope.close() is called.

Returns:
Scope to control the end of the active period for
Expand Down Expand Up @@ -112,22 +115,22 @@ class _LogContextScope(Scope):
def __init__(
self,
manager: LogContextScopeManager,
span,
logcontext,
span: Span,
logcontext: LoggingContext,
enter_logcontext: bool,
finish_on_close: bool,
):
"""
Args:
manager:
the manager that is responsible for this scope.
span (Span):
span:
the opentracing span which this scope represents the local
lifetime for.
logcontext (LogContext):
the logcontext to which this scope is attached.
logcontext:
the log context to which this scope is attached.
enter_logcontext:
if True the logcontext will be exited when the scope is finished
if True the log context will be exited when the scope is finished
finish_on_close:
if True finish the span when the scope is closed
"""
Expand Down
2 changes: 1 addition & 1 deletion tests/logging/test_opentracing.py
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ def setUp(self) -> None:
# global variables that power opentracing. We create our own tracer instance
# and test with it.

scope_manager = LogContextScopeManager({})
scope_manager = LogContextScopeManager()
config = jaeger_client.config.Config(
config={}, service_name="test", scope_manager=scope_manager
)
Expand Down