Skip to content

Commit

Permalink
Add opentracing types (matrix-org#11603)
Browse files Browse the repository at this point in the history
  • Loading branch information
H-Shay authored Dec 20, 2021
1 parent 8428ef6 commit 8ad3943
Show file tree
Hide file tree
Showing 5 changed files with 18 additions and 13 deletions.
1 change: 1 addition & 0 deletions changelog.d/11603.misc
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Add opentracing type stubs and fix associated mypy errors.
3 changes: 0 additions & 3 deletions mypy.ini
Original file line number Diff line number Diff line change
Expand Up @@ -308,9 +308,6 @@ ignore_missing_imports = True
[mypy-netaddr]
ignore_missing_imports = True

[mypy-opentracing]
ignore_missing_imports = True

[mypy-parameterized.*]
ignore_missing_imports = True

Expand Down
1 change: 1 addition & 0 deletions setup.py
Original file line number Diff line number Diff line change
Expand Up @@ -107,6 +107,7 @@ def exec_file(path_segments):
"mypy-zope==0.3.2",
"types-bleach>=4.1.0",
"types-jsonschema>=3.2.0",
"types-opentracing>=2.4.2",
"types-Pillow>=8.3.4",
"types-pyOpenSSL>=20.0.7",
"types-PyYAML>=5.4.10",
Expand Down
24 changes: 15 additions & 9 deletions synapse/logging/opentracing.py
Original file line number Diff line number Diff line change
Expand Up @@ -222,8 +222,8 @@ class _DummyTagNames:

tags = opentracing.tags
except ImportError:
opentracing = None
tags = _DummyTagNames
opentracing = None # type: ignore[assignment]
tags = _DummyTagNames # type: ignore[assignment]
try:
from jaeger_client import Config as JaegerConfig

Expand Down Expand Up @@ -366,7 +366,7 @@ def init_tracer(hs: "HomeServer"):
global opentracing
if not hs.config.tracing.opentracer_enabled:
# We don't have a tracer
opentracing = None
opentracing = None # type: ignore[assignment]
return

if not opentracing or not JaegerConfig:
Expand Down Expand Up @@ -452,7 +452,7 @@ def start_active_span(
"""

if opentracing is None:
return noop_context_manager()
return noop_context_manager() # type: ignore[unreachable]

return opentracing.tracer.start_active_span(
operation_name,
Expand All @@ -477,7 +477,7 @@ def start_active_span_follows_from(
forced, the new span will also have tracing forced.
"""
if opentracing is None:
return noop_context_manager()
return noop_context_manager() # type: ignore[unreachable]

references = [opentracing.follows_from(context) for context in contexts]
scope = start_active_span(operation_name, references=references)
Expand Down Expand Up @@ -514,7 +514,7 @@ def start_active_span_from_request(
# Also, twisted uses byte arrays while opentracing expects strings.

if opentracing is None:
return noop_context_manager()
return noop_context_manager() # type: ignore[unreachable]

header_dict = {
k.decode(): v[0].decode() for k, v in request.requestHeaders.getAllRawHeaders()
Expand Down Expand Up @@ -553,7 +553,7 @@ def start_active_span_from_edu(
references = references or []

if opentracing is None:
return noop_context_manager()
return noop_context_manager() # type: ignore[unreachable]

carrier = json_decoder.decode(edu_content.get("context", "{}")).get(
"opentracing", {}
Expand Down Expand Up @@ -594,18 +594,21 @@ def active_span():
@ensure_active_span("set a tag")
def set_tag(key, value):
"""Sets a tag on the active span"""
assert opentracing.tracer.active_span is not None
opentracing.tracer.active_span.set_tag(key, value)


@ensure_active_span("log")
def log_kv(key_values, timestamp=None):
"""Log to the active span"""
assert opentracing.tracer.active_span is not None
opentracing.tracer.active_span.log_kv(key_values, timestamp)


@ensure_active_span("set the traces operation name")
def set_operation_name(operation_name):
"""Sets the operation name of the active span"""
assert opentracing.tracer.active_span is not None
opentracing.tracer.active_span.set_operation_name(operation_name)


Expand Down Expand Up @@ -674,6 +677,7 @@ def inject_header_dict(
span = opentracing.tracer.active_span

carrier: Dict[str, str] = {}
assert span is not None
opentracing.tracer.inject(span.context, opentracing.Format.HTTP_HEADERS, carrier)

for key, value in carrier.items():
Expand Down Expand Up @@ -716,6 +720,7 @@ def get_active_span_text_map(destination=None):
return {}

carrier: Dict[str, str] = {}
assert opentracing.tracer.active_span is not None
opentracing.tracer.inject(
opentracing.tracer.active_span.context, opentracing.Format.TEXT_MAP, carrier
)
Expand All @@ -731,6 +736,7 @@ def active_span_context_as_string():
"""
carrier: Dict[str, str] = {}
if opentracing:
assert opentracing.tracer.active_span is not None
opentracing.tracer.inject(
opentracing.tracer.active_span.context, opentracing.Format.TEXT_MAP, carrier
)
Expand Down Expand Up @@ -773,7 +779,7 @@ def trace(func=None, opname=None):

def decorator(func):
if opentracing is None:
return func
return func # type: ignore[unreachable]

_opname = opname if opname else func.__name__

Expand Down Expand Up @@ -864,7 +870,7 @@ def trace_servlet(request: "SynapseRequest", extract_context: bool = False):
"""

if opentracing is None:
yield
yield # type: ignore[unreachable]
return

request_tags = {
Expand Down
2 changes: 1 addition & 1 deletion synapse/logging/scopecontextmanager.py
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,7 @@ def activate(self, span, finish_on_close):
if not ctx:
# We don't want this scope to affect.
logger.error("Tried to activate scope outside of loggingcontext")
return Scope(None, span)
return Scope(None, span) # type: ignore[arg-type]
elif ctx.scope is not None:
# We want the logging scope to look exactly the same so we give it
# a blank suffix
Expand Down

0 comments on commit 8ad3943

Please sign in to comment.