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
53 changes: 37 additions & 16 deletions synapse/api/auth.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@
from synapse.events import EventBase
from synapse.http import get_request_user_agent
from synapse.http.site import SynapseRequest
from synapse.logging import opentracing as opentracing
from synapse.logging.opentracing import active_span, force_tracing, start_active_span
from synapse.storage.databases.main.registration import TokenLookupResult
from synapse.types import Requester, StateMap, UserID, create_requester
from synapse.util.caches.lrucache import LruCache
Expand Down Expand Up @@ -149,6 +149,42 @@ async def get_user_by_req(
is invalid.
AuthError if access is denied for the user in the access token
"""
parent_span = active_span()
with start_active_span("get_user_by_req"):
requester = await self._wrapped_get_user_by_req(
request, allow_guest, rights, allow_expired
)

if parent_span:
if requester.authenticated_entity in self._force_tracing_for_users:
# request tracing is enabled for this user, so we need to force it
# tracing on for the parent span (which will be the servlet span).
#
# It's too late for the get_user_by_req span to inherit the setting,
# so we also force it on for that.
force_tracing()
force_tracing(parent_span)
parent_span.set_tag(
"authenticated_entity", requester.authenticated_entity
)
Comment on lines +167 to +169
Copy link
Member

Choose a reason for hiding this comment

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

I had to convince myself that authenticated_entity was the same here:

  • For appservices authenticated_entity isn't set during creation of the requester and we report the tag as user_id; this was essentially inlined logic from create_requester
  • For non-appservices, this was set to user_info.token_owner, which is the same thing that is passed as the authenticated_entity in create_requester.

Copy link
Member Author

Choose a reason for hiding this comment

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

sorry, yes, I must have gone through that same thought process, but forgot to write it down here!

Copy link
Member

Choose a reason for hiding this comment

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

No problem! Definitely better to put all this logic in one place though!

parent_span.set_tag("user_id", requester.user.to_string())
if requester.device_id is not None:
parent_span.set_tag("device_id", requester.device_id)
if requester.app_service is not None:
parent_span.set_tag("appservice_id", requester.app_service.id)
return requester

async def _wrapped_get_user_by_req(
self,
request: SynapseRequest,
allow_guest: bool,
rights: str,
allow_expired: bool,
) -> Requester:
"""Helper for get_user_by_req

Once get_user_by_req has set up the opentracing span, this does the actual work.
"""
try:
ip_addr = request.getClientIP()
user_agent = get_request_user_agent(request)
Expand Down Expand Up @@ -177,14 +213,6 @@ async def get_user_by_req(
)

request.requester = user_id
if user_id in self._force_tracing_for_users:
opentracing.force_tracing()
opentracing.set_tag("authenticated_entity", user_id)
opentracing.set_tag("user_id", user_id)
if device_id is not None:
opentracing.set_tag("device_id", device_id)
opentracing.set_tag("appservice_id", app_service.id)

return requester

user_info = await self.get_user_by_access_token(
Expand Down Expand Up @@ -242,13 +270,6 @@ async def get_user_by_req(
)

request.requester = requester
if user_info.token_owner in self._force_tracing_for_users:
opentracing.force_tracing()
opentracing.set_tag("authenticated_entity", user_info.token_owner)
opentracing.set_tag("user_id", user_info.user_id)
if device_id:
opentracing.set_tag("device_id", device_id)

return requester
except KeyError:
raise MissingClientTokenError()
Expand Down