diff options
| -rw-r--r-- | changelog.d/10143.misc | 1 | ||||
| -rw-r--r-- | synapse/http/matrixfederationclient.py | 10 | ||||
| -rw-r--r-- | synapse/logging/opentracing.py | 102 | ||||
| -rw-r--r-- | synapse/replication/http/_base.py | 5 |
4 files changed, 26 insertions, 92 deletions
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 |