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

Commit ceddc87

Browse files
committed
Improve opentracing annotations for Notifier
The existing tracing reports an error each time there is a timeout, which isn't really representative. Additionally, we log things about the way `wait_for_events` works (eg, the result of the callback) to the *parent* span, which is confusing.
1 parent ed53bf3 commit ceddc87

File tree

2 files changed

+34
-33
lines changed

2 files changed

+34
-33
lines changed

changelog.d/10102.misc

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
Improve opentracing annotations for `Notifier`.

synapse/notifier.py

Lines changed: 33 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -485,21 +485,21 @@ async def wait_for_events(
485485
end_time = self.clock.time_msec() + timeout
486486

487487
while not result:
488-
try:
489-
now = self.clock.time_msec()
490-
if end_time <= now:
491-
break
492-
493-
# Now we wait for the _NotifierUserStream to be told there
494-
# is a new token.
495-
listener = user_stream.new_listener(prev_token)
496-
listener.deferred = timeout_deferred(
497-
listener.deferred,
498-
(end_time - now) / 1000.0,
499-
self.hs.get_reactor(),
500-
)
488+
with start_active_span("wait_for_events"):
489+
try:
490+
now = self.clock.time_msec()
491+
if end_time <= now:
492+
break
493+
494+
# Now we wait for the _NotifierUserStream to be told there
495+
# is a new token.
496+
listener = user_stream.new_listener(prev_token)
497+
listener.deferred = timeout_deferred(
498+
listener.deferred,
499+
(end_time - now) / 1000.0,
500+
self.hs.get_reactor(),
501+
)
501502

502-
with start_active_span("wait_for_events.deferred"):
503503
log_kv(
504504
{
505505
"wait_for_events": "sleep",
@@ -517,27 +517,27 @@ async def wait_for_events(
517517
}
518518
)
519519

520-
current_token = user_stream.current_token
520+
current_token = user_stream.current_token
521521

522-
result = await callback(prev_token, current_token)
523-
log_kv(
524-
{
525-
"wait_for_events": "result",
526-
"result": bool(result),
527-
}
528-
)
529-
if result:
522+
result = await callback(prev_token, current_token)
523+
log_kv(
524+
{
525+
"wait_for_events": "result",
526+
"result": bool(result),
527+
}
528+
)
529+
if result:
530+
break
531+
532+
# Update the prev_token to the current_token since nothing
533+
# has happened between the old prev_token and the current_token
534+
prev_token = current_token
535+
except defer.TimeoutError:
536+
log_kv({"wait_for_events": "timeout"})
537+
break
538+
except defer.CancelledError:
539+
log_kv({"wait_for_events": "cancelled"})
530540
break
531-
532-
# Update the prev_token to the current_token since nothing
533-
# has happened between the old prev_token and the current_token
534-
prev_token = current_token
535-
except defer.TimeoutError:
536-
log_kv({"wait_for_events": "timeout"})
537-
break
538-
except defer.CancelledError:
539-
log_kv({"wait_for_events": "cancelled"})
540-
break
541541

542542
if result is None:
543543
# This happened if there was no timeout or if the timeout had

0 commit comments

Comments
 (0)