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

Report per request metrics for all of the things using request_handler #756

Merged
merged 3 commits into from
Apr 28, 2016
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
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
101 changes: 63 additions & 38 deletions synapse/http/server.py
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,12 @@
_next_request_id = 0


def request_handler(request_handler):
def request_handler(report_metrics=True):
"""Decorator for ``wrap_request_handler``"""
return lambda request_handler: wrap_request_handler(request_handler, report_metrics)


def wrap_request_handler(request_handler, report_metrics):
"""Wraps a method that acts as a request handler with the necessary logging
and exception handling.

Expand All @@ -96,6 +101,10 @@ def wrapped_request_handler(self, request):
global _next_request_id
request_id = "%s-%s" % (request.method, _next_request_id)
_next_request_id += 1
if report_metrics:
request_metrics = RequestMetrics()
request_metrics.start(self.clock)

with LoggingContext(request_id) as request_context:
request_context.request = request_id
with request.processing():
Expand Down Expand Up @@ -133,6 +142,13 @@ def wrapped_request_handler(self, request):
},
send_cors=True
)
finally:
try:
request_metrics.stop(
self.clock, request, self.__class__.__name__
)
except:
Copy link
Member

Choose a reason for hiding this comment

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

Ewwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwww.

Can we not instead just check if report_metrics is true?

pass
return wrapped_request_handler


Expand Down Expand Up @@ -197,19 +213,19 @@ def render(self, request):
self._async_render(request)
return server.NOT_DONE_YET

@request_handler
@request_handler(report_metrics=False)
Copy link
Member

Choose a reason for hiding this comment

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

Why can this not be True?

@defer.inlineCallbacks
def _async_render(self, request):
""" This gets called from render() every time someone sends us a request.
This checks if anyone has registered a callback for that method and
path.
"""
start = self.clock.time_msec()
if request.method == "OPTIONS":
self._send_response(request, 200, {})
return

start_context = LoggingContext.current_context()
request_metrics = RequestMetrics()
request_metrics.start(self.clock)

# Loop through all the registered callbacks to check if the method
# and path regex match
Expand Down Expand Up @@ -241,40 +257,7 @@ def _async_render(self, request):
self._send_response(request, code, response)

try:
context = LoggingContext.current_context()

tag = ""
if context:
tag = context.tag

if context != start_context:
logger.warn(
"Context have unexpectedly changed %r, %r",
context, self.start_context
)
return

incoming_requests_counter.inc(request.method, servlet_classname, tag)

response_timer.inc_by(
self.clock.time_msec() - start, request.method,
servlet_classname, tag
)

ru_utime, ru_stime = context.get_resource_usage()

response_ru_utime.inc_by(
ru_utime, request.method, servlet_classname, tag
)
response_ru_stime.inc_by(
ru_stime, request.method, servlet_classname, tag
)
response_db_txn_count.inc_by(
context.db_txn_count, request.method, servlet_classname, tag
)
response_db_txn_duration.inc_by(
context.db_txn_duration, request.method, servlet_classname, tag
)
request_metrics.stop(self.clock, request, servlet_classname)
except:
pass

Expand Down Expand Up @@ -307,6 +290,48 @@ def _send_response(self, request, code, response_json_object,
)


class RequestMetrics(object):
def start(self, clock):
self.start = clock.time_msec()
self.start_context = LoggingContext.current_context()

def stop(self, clock, request, servlet_classname):
context = LoggingContext.current_context()

tag = ""
if context:
tag = context.tag

if context != start_context:
logger.warn(
"Context have unexpectedly changed %r, %r",
context, self.start_context
)
return

incoming_requests_counter.inc(request.method, servlet_classname, tag)

response_timer.inc_by(
self.clock.time_msec() - start, request.method,
servlet_classname, tag
)

ru_utime, ru_stime = context.get_resource_usage()

response_ru_utime.inc_by(
ru_utime, request.method, servlet_classname, tag
)
response_ru_stime.inc_by(
ru_stime, request.method, servlet_classname, tag
)
response_db_txn_count.inc_by(
context.db_txn_count, request.method, servlet_classname, tag
)
response_db_txn_duration.inc_by(
context.db_txn_duration, request.method, servlet_classname, tag
)


class RootRedirect(resource.Resource):
"""Redirects the root '/' path to another path."""

Expand Down
3 changes: 2 additions & 1 deletion synapse/replication/pusher_resource.py
Original file line number Diff line number Diff line change
Expand Up @@ -31,12 +31,13 @@ def __init__(self, hs):
self.version_string = hs.version_string
self.store = hs.get_datastore()
self.notifier = hs.get_notifier()
self.clock = hs.get_clock()

def render_POST(self, request):
self._async_render_POST(request)
return NOT_DONE_YET

@request_handler
@request_handler()
@defer.inlineCallbacks
def _async_render_POST(self, request):
content = parse_json_object_from_request(request)
Expand Down
3 changes: 2 additions & 1 deletion synapse/replication/resource.py
Original file line number Diff line number Diff line change
Expand Up @@ -112,6 +112,7 @@ def __init__(self, hs):
self.presence_handler = hs.get_handlers().presence_handler
self.typing_handler = hs.get_handlers().typing_notification_handler
self.notifier = hs.notifier
self.clock = hs.get_clock()

self.putChild("remove_pushers", PusherResource(hs))

Expand Down Expand Up @@ -139,7 +140,7 @@ def current_replication_token(self):
state_token,
))

@request_handler
@request_handler()
@defer.inlineCallbacks
def _async_render_GET(self, request):
limit = parse_integer(request, "limit", 100)
Expand Down
1 change: 0 additions & 1 deletion synapse/rest/key/v1/server_key_resource.py
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,6 @@ class LocalKey(Resource):
"""

def __init__(self, hs):
self.hs = hs
self.version_string = hs.version_string
self.response_body = encode_canonical_json(
self.response_json_object(hs.config)
Expand Down
4 changes: 2 additions & 2 deletions synapse/rest/key/v2/remote_key_resource.py
Original file line number Diff line number Diff line change
Expand Up @@ -97,7 +97,7 @@ def render_GET(self, request):
self.async_render_GET(request)
return NOT_DONE_YET

@request_handler
@request_handler()
@defer.inlineCallbacks
def async_render_GET(self, request):
if len(request.postpath) == 1:
Expand All @@ -122,7 +122,7 @@ def render_POST(self, request):
self.async_render_POST(request)
return NOT_DONE_YET

@request_handler
@request_handler()
@defer.inlineCallbacks
def async_render_POST(self, request):
content = parse_json_object_from_request(request)
Expand Down
3 changes: 2 additions & 1 deletion synapse/rest/media/v1/download_resource.py
Original file line number Diff line number Diff line change
Expand Up @@ -36,12 +36,13 @@ def __init__(self, hs, media_repo):
self.server_name = hs.hostname
self.store = hs.get_datastore()
self.version_string = hs.version_string
self.clock = hs.get_clock()

def render_GET(self, request):
self._async_render_GET(request)
return NOT_DONE_YET

@request_handler
@request_handler()
@defer.inlineCallbacks
def _async_render_GET(self, request):
server_name, media_id, name = parse_media_id(request)
Expand Down
2 changes: 1 addition & 1 deletion synapse/rest/media/v1/preview_url_resource.py
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,7 @@ def render_GET(self, request):
self._async_render_GET(request)
return NOT_DONE_YET

@request_handler
@request_handler()
@defer.inlineCallbacks
def _async_render_GET(self, request):

Expand Down
3 changes: 2 additions & 1 deletion synapse/rest/media/v1/thumbnail_resource.py
Original file line number Diff line number Diff line change
Expand Up @@ -39,12 +39,13 @@ def __init__(self, hs, media_repo):
self.dynamic_thumbnails = hs.config.dynamic_thumbnails
self.server_name = hs.hostname
self.version_string = hs.version_string
self.clock = hs.get_clock()

def render_GET(self, request):
self._async_render_GET(request)
return NOT_DONE_YET

@request_handler
@request_handler()
@defer.inlineCallbacks
def _async_render_GET(self, request):
server_name, media_id, _ = parse_media_id(request)
Expand Down
3 changes: 2 additions & 1 deletion synapse/rest/media/v1/upload_resource.py
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@ def __init__(self, hs, media_repo):
self.auth = hs.get_auth()
self.max_upload_size = hs.config.max_upload_size
self.version_string = hs.version_string
self.clock = hs.get_clock()

def render_POST(self, request):
self._async_render_POST(request)
Expand All @@ -50,7 +51,7 @@ def render_OPTIONS(self, request):
respond_with_json(request, 200, {}, send_cors=True)
return NOT_DONE_YET

@request_handler
@request_handler()
@defer.inlineCallbacks
def _async_render_POST(self, request):
requester = yield self.auth.get_user_by_req(request)
Expand Down