From 4af31233cc9a0ee6f01647557e403a9dae561934 Mon Sep 17 00:00:00 2001 From: Richard van der Hoff Date: Tue, 8 Jun 2021 17:45:52 +0100 Subject: [PATCH 1/3] Remove unused helper functions --- synapse/logging/opentracing.py | 68 ---------------------------------- 1 file changed, 68 deletions(-) diff --git a/synapse/logging/opentracing.py b/synapse/logging/opentracing.py index 26c8ffe780eb..1826a802286a 100644 --- a/synapse/logging/opentracing.py +++ b/synapse/logging/opentracing.py @@ -573,42 +573,6 @@ 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): - """ - Injects a span context into twisted headers in-place - - 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 - 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): """ @@ -646,38 +610,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): """ From 6b43f6b1bc421adab395aac262855e9ea59ba42b Mon Sep 17 00:00:00 2001 From: Richard van der Hoff Date: Tue, 8 Jun 2021 17:47:24 +0100 Subject: [PATCH 2/3] Clean up the interface for injecting opentracing over HTTP --- synapse/http/matrixfederationclient.py | 10 +++----- synapse/logging/opentracing.py | 34 ++++++++++++++------------ synapse/replication/http/_base.py | 5 ++-- 3 files changed, 25 insertions(+), 24 deletions(-) diff --git a/synapse/http/matrixfederationclient.py b/synapse/http/matrixfederationclient.py index 1998990a144e..629373fc475b 100644 --- a/synapse/http/matrixfederationclient.py +++ b/synapse/http/matrixfederationclient.py @@ -65,13 +65,9 @@ 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 @@ async def _send_request( # 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 1826a802286a..6b8ec97c3958 100644 --- a/synapse/logging/opentracing.py +++ b/synapse/logging/opentracing.py @@ -168,7 +168,7 @@ def set_fates(clotho, lachesis, atropos, father="Zues", mother="Themis"): 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 @@ -573,23 +573,22 @@ def set_operation_name(operation_name): # Injection and extraction -@ensure_active_span("inject the span into a byte dict") -def inject_active_span_byte_dict(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 a dict where the headers are encoded as byte - strings + Injects a span context into a dict of HTTP headers 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 + 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 @@ -598,8 +597,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 diff --git a/synapse/replication/http/_base.py b/synapse/replication/http/_base.py index 5685cf212144..2a13026e9a16 100644 --- a/synapse/replication/http/_base.py +++ b/synapse/replication/http/_base.py @@ -23,7 +23,8 @@ 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 @@ async def send_request(*, instance_name="master", **kwargs): # 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 From 3d1e15fe3ccbbf1836dfcc29fdff677a4a9493c3 Mon Sep 17 00:00:00 2001 From: Richard van der Hoff Date: Tue, 8 Jun 2021 17:47:57 +0100 Subject: [PATCH 3/3] changelog --- changelog.d/10143.misc | 1 + 1 file changed, 1 insertion(+) create mode 100644 changelog.d/10143.misc diff --git a/changelog.d/10143.misc b/changelog.d/10143.misc new file mode 100644 index 000000000000..37aa344db2d8 --- /dev/null +++ b/changelog.d/10143.misc @@ -0,0 +1 @@ +Clean up the interface for injecting opentracing over HTTP.