diff --git a/changelog.d/10143.misc b/changelog.d/10143.misc
new file mode 100644
index 0000000000..37aa344db2
--- /dev/null
+++ b/changelog.d/10143.misc
@@ -0,0 +1 @@
+Clean up the interface for injecting opentracing over HTTP.
diff --git a/synapse/http/matrixfederationclient.py b/synapse/http/matrixfederationclient.py
index 1998990a14..629373fc47 100644
--- a/synapse/http/matrixfederationclient.py
+++ b/synapse/http/matrixfederationclient.py
@@ -65,13 +65,9 @@ from synapse.http.client import (
read_body_with_max_size,
)
from synapse.http.federation.matrix_federation_agent import MatrixFederationAgent
+from synapse.logging import opentracing
from synapse.logging.context import make_deferred_yieldable
-from synapse.logging.opentracing import (
- inject_active_span_byte_dict,
- set_tag,
- start_active_span,
- tags,
-)
+from synapse.logging.opentracing import set_tag, start_active_span, tags
from synapse.types import ISynapseReactor, JsonDict
from synapse.util import json_decoder
from synapse.util.async_helpers import timeout_deferred
@@ -497,7 +493,7 @@ class MatrixFederationHttpClient:
# Inject the span into the headers
headers_dict = {} # type: Dict[bytes, List[bytes]]
- inject_active_span_byte_dict(headers_dict, request.destination)
+ opentracing.inject_header_dict(headers_dict, request.destination)
headers_dict[b"User-Agent"] = [self.version_string_bytes]
diff --git a/synapse/logging/opentracing.py b/synapse/logging/opentracing.py
index dd9377340e..5b4725e035 100644
--- a/synapse/logging/opentracing.py
+++ b/synapse/logging/opentracing.py
@@ -168,7 +168,7 @@ import inspect
import logging
import re
from functools import wraps
-from typing import TYPE_CHECKING, Dict, Optional, Pattern, Type
+from typing import TYPE_CHECKING, Dict, List, Optional, Pattern, Type
import attr
@@ -574,59 +574,22 @@ def set_operation_name(operation_name):
# Injection and extraction
-@ensure_active_span("inject the span into a header")
-def inject_active_span_twisted_headers(headers, destination, check_destination=True):
+@ensure_active_span("inject the span into a header dict")
+def inject_header_dict(
+ headers: Dict[bytes, List[bytes]],
+ destination: Optional[str] = None,
+ check_destination: bool = True,
+) -> None:
"""
- Injects a span context into twisted headers in-place
+ Injects a span context into a dict of HTTP headers
Args:
- headers (twisted.web.http_headers.Headers)
- destination (str): address of entity receiving the span context. If check_destination
- is true the context will only be injected if the destination matches the
- opentracing whitelist
+ headers: the dict to inject headers into
+ destination: address of entity receiving the span context. Must be given unless
+ check_destination is False. The context will only be injected if the
+ destination matches the opentracing whitelist
check_destination (bool): If false, destination will be ignored and the context
will always be injected.
- span (opentracing.Span)
-
- Returns:
- In-place modification of headers
-
- Note:
- The headers set by the tracer are custom to the tracer implementation which
- should be unique enough that they don't interfere with any headers set by
- synapse or twisted. If we're still using jaeger these headers would be those
- here:
- https://github.com/jaegertracing/jaeger-client-python/blob/master/jaeger_client/constants.py
- """
-
- if check_destination and not whitelisted_homeserver(destination):
- return
-
- span = opentracing.tracer.active_span
- carrier = {} # type: Dict[str, str]
- opentracing.tracer.inject(span.context, opentracing.Format.HTTP_HEADERS, carrier)
-
- for key, value in carrier.items():
- headers.addRawHeaders(key, value)
-
-
-@ensure_active_span("inject the span into a byte dict")
-def inject_active_span_byte_dict(headers, destination, check_destination=True):
- """
- Injects a span context into a dict where the headers are encoded as byte
- strings
-
- Args:
- headers (dict)
- destination (str): address of entity receiving the span context. If check_destination
- is true the context will only be injected if the destination matches the
- opentracing whitelist
- check_destination (bool): If false, destination will be ignored and the context
- will always be injected.
- span (opentracing.Span)
-
- Returns:
- In-place modification of headers
Note:
The headers set by the tracer are custom to the tracer implementation which
@@ -635,8 +598,13 @@ def inject_active_span_byte_dict(headers, destination, check_destination=True):
here:
https://github.com/jaegertracing/jaeger-client-python/blob/master/jaeger_client/constants.py
"""
- if check_destination and not whitelisted_homeserver(destination):
- return
+ if check_destination:
+ if destination is None:
+ raise ValueError(
+ "destination must be given unless check_destination is False"
+ )
+ if not whitelisted_homeserver(destination):
+ return
span = opentracing.tracer.active_span
@@ -647,38 +615,6 @@ def inject_active_span_byte_dict(headers, destination, check_destination=True):
headers[key.encode()] = [value.encode()]
-@ensure_active_span("inject the span into a text map")
-def inject_active_span_text_map(carrier, destination, check_destination=True):
- """
- Injects a span context into a dict
-
- Args:
- carrier (dict)
- destination (str): address of entity receiving the span context. If check_destination
- is true the context will only be injected if the destination matches the
- opentracing whitelist
- check_destination (bool): If false, destination will be ignored and the context
- will always be injected.
-
- Returns:
- In-place modification of carrier
-
- Note:
- The headers set by the tracer are custom to the tracer implementation which
- should be unique enough that they don't interfere with any headers set by
- synapse or twisted. If we're still using jaeger these headers would be those
- here:
- https://github.com/jaegertracing/jaeger-client-python/blob/master/jaeger_client/constants.py
- """
-
- if check_destination and not whitelisted_homeserver(destination):
- return
-
- opentracing.tracer.inject(
- opentracing.tracer.active_span.context, opentracing.Format.TEXT_MAP, carrier
- )
-
-
@ensure_active_span("get the active span context as a dict", ret={})
def get_active_span_text_map(destination=None):
"""
diff --git a/synapse/replication/http/_base.py b/synapse/replication/http/_base.py
index 5685cf2121..2a13026e9a 100644
--- a/synapse/replication/http/_base.py
+++ b/synapse/replication/http/_base.py
@@ -23,7 +23,8 @@ from prometheus_client import Counter, Gauge
from synapse.api.errors import HttpResponseException, SynapseError
from synapse.http import RequestTimedOutError
-from synapse.logging.opentracing import inject_active_span_byte_dict, trace
+from synapse.logging import opentracing
+from synapse.logging.opentracing import trace
from synapse.util.caches.response_cache import ResponseCache
from synapse.util.stringutils import random_string
@@ -235,7 +236,7 @@ class ReplicationEndpoint(metaclass=abc.ABCMeta):
# Add an authorization header, if configured.
if replication_secret:
headers[b"Authorization"] = [b"Bearer " + replication_secret]
- inject_active_span_byte_dict(headers, None, check_destination=False)
+ opentracing.inject_header_dict(headers, check_destination=False)
try:
result = await request_func(uri, data, headers=headers)
break
|
