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

Various opentracing enhancements #11619

Merged
merged 8 commits into from
Dec 21, 2021
Merged

Various opentracing enhancements #11619

Changes from 1 commit
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
9aee261
Wrap `auth.get_user_by_req` in an opentracing span
richvdh Dec 20, 2021
0891776
Emit opentracing span for encoding json responses
richvdh Dec 17, 2021
68cd102
Rename all sync spans with a prefix
richvdh Dec 17, 2021
41fab2a
Write an opentracing span for encoding sync response
richvdh Dec 17, 2021
c95b8ef
opentracing span to group generate_room_entries
richvdh Dec 17, 2021
5676701
opentracing spans within sync.encode_response
richvdh Dec 17, 2021
aaf59c8
changelog
richvdh Dec 20, 2021
fd7b6a0
Use the `trace` decorator instead of context managers
richvdh Dec 21, 2021
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
19 changes: 17 additions & 2 deletions synapse/http/server.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -58,12 +58,14 @@
)
from synapse.http.site import SynapseRequest
from synapse.logging.context import defer_to_thread, preserve_fn, run_in_background
from synapse.logging.opentracing import trace_servlet
from synapse.logging.opentracing import active_span, start_active_span, trace_servlet
from synapse.util import json_encoder
from synapse.util.caches import intern_dict
from synapse.util.iterutils import chunk_seq

if TYPE_CHECKING:
import opentracing

from synapse.server import HomeServer

logger = logging.getLogger(__name__)
Expand Down Expand Up @@ -759,7 +761,20 @@ async def _async_write_json_to_request_in_thread(
expensive.
"""

json_str = await defer_to_thread(request.reactor, json_encoder, json_object)
def encode(opentracing_span: "Optional[opentracing.Span]") -> bytes:
# it might take a while for the threadpool to schedule us, so we write
# opentracing logs once we actually get scheduled, so that we can see how
# much that contributed.
if opentracing_span:
opentracing_span.log_kv({"event": "scheduled"})
res = json_encoder(json_object)
if opentracing_span:
opentracing_span.log_kv({"event": "encoded"})
return res

with start_active_span("encode_json_response"):
span = active_span()
Comment on lines +775 to +776
Copy link
Member

Choose a reason for hiding this comment

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

We seem to be using this same logic in a bunch of places, it looks similar to the trace decorator, but not the same. It feels really duplicative, but maybe that's just open tracing?

(I guess in this case we need the span itself so we can call log_kv on it...)

Copy link
Member Author

Choose a reason for hiding this comment

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

well, it's less opentracing, and more our wrappers for opentracing (which make opentracing itself an optional dependency). If you have a real opentracing, you can do:

    with start_active_span("encode_json_response") as scope:
        span = scope.span
        span.log_kv(...)

... but with our wrappers, scope can be None, so you need the extra active_span() call.

Copy link
Member

Choose a reason for hiding this comment

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

Ah, OK! That makes a bit more sense!

json_str = await defer_to_thread(request.reactor, encode, span)

_write_bytes_to_request(request, json_str)

Expand Down