-
Notifications
You must be signed in to change notification settings - Fork 647
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
metric instrumentation Tornado #1252
Changes from 23 commits
90d23d7
4d52bbb
c336916
f5369ed
df4dcf8
cd51b99
7fe024f
697701b
df49a28
d70acc2
033c62c
eea4f40
340b296
3ab38f0
f579964
d1f5f43
9f24abf
28538de
4531588
eecca87
421b269
ae128d4
af85f32
5c0e8bd
6044ab2
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -157,7 +157,8 @@ def client_resposne_hook(span, future): | |
from functools import partial | ||
from logging import getLogger | ||
from time import time_ns | ||
from typing import Collection | ||
from timeit import default_timer | ||
from typing import Collection, Dict | ||
|
||
import tornado.web | ||
import wrapt | ||
|
@@ -177,6 +178,8 @@ def client_resposne_hook(span, future): | |
http_status_to_status_code, | ||
unwrap, | ||
) | ||
from opentelemetry.metrics import get_meter | ||
from opentelemetry.metrics._internal.instrument import Histogram | ||
from opentelemetry.propagators import textmap | ||
from opentelemetry.semconv.trace import SpanAttributes | ||
from opentelemetry.trace.status import Status, StatusCode | ||
|
@@ -198,6 +201,12 @@ def client_resposne_hook(span, future): | |
_OTEL_PATCHED_KEY = "_otel_patched_key" | ||
|
||
|
||
_START_TIME = "start_time" | ||
_SERVER_DURATION_HISTOGRAM = "http.server.duration" | ||
_SERVER_REQUEST_SIZE_HISTOGRAM = "http.server.request.size" | ||
_SERVER_RESPONSE_SIZE_HISTOGRAM = "http.server.response.size" | ||
_SERVER_ACTIVE_REQUESTS_HISTOGRAM = "http.server.active_requests" | ||
|
||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Maybe add the constants for client up here as well? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Fixed |
||
_excluded_urls = get_excluded_urls("TORNADO") | ||
_traced_request_attrs = get_traced_request_attrs("TORNADO") | ||
response_propagation_setter = FuncSetter(tornado.web.RequestHandler.add_header) | ||
|
@@ -233,13 +242,36 @@ def _instrument(self, **kwargs): | |
tracer_provider = kwargs.get("tracer_provider") | ||
tracer = trace.get_tracer(__name__, __version__, tracer_provider) | ||
|
||
meter_provider = kwargs.get("meter_provider") | ||
meter = get_meter(__name__, __version__, meter_provider) | ||
|
||
server_histograms = _create_server_histograms(meter) | ||
|
||
client_duration_histogram = meter.create_histogram( | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Perhaps you can refactor to use There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Fixed |
||
name="http.client.duration", | ||
unit="ms", | ||
description="measures the duration outbound HTTP requests", | ||
) | ||
client_request_size_histogram = meter.create_histogram( | ||
name="http.client.request.size", | ||
unit="By", | ||
description="measures the size of HTTP request messages (compressed)", | ||
) | ||
client_response_size_histogram = meter.create_histogram( | ||
name="http.client.response.size", | ||
unit="By", | ||
description="measures the size of HTTP response messages (compressed)", | ||
) | ||
|
||
lzchen marked this conversation as resolved.
Show resolved
Hide resolved
|
||
client_request_hook = kwargs.get("client_request_hook", None) | ||
client_response_hook = kwargs.get("client_response_hook", None) | ||
server_request_hook = kwargs.get("server_request_hook", None) | ||
|
||
def handler_init(init, handler, args, kwargs): | ||
cls = handler.__class__ | ||
if patch_handler_class(tracer, cls, server_request_hook): | ||
if patch_handler_class( | ||
tracer, server_histograms, cls, server_request_hook | ||
): | ||
self.patched_handlers.append(cls) | ||
return init(*args, **kwargs) | ||
|
||
|
@@ -250,7 +282,13 @@ def handler_init(init, handler, args, kwargs): | |
"tornado.httpclient", | ||
"AsyncHTTPClient.fetch", | ||
partial( | ||
fetch_async, tracer, client_request_hook, client_response_hook | ||
fetch_async, | ||
tracer, | ||
client_request_hook, | ||
client_response_hook, | ||
client_duration_histogram, | ||
client_request_size_histogram, | ||
client_response_size_histogram, | ||
), | ||
) | ||
|
||
|
@@ -262,14 +300,49 @@ def _uninstrument(self, **kwargs): | |
self.patched_handlers = [] | ||
|
||
|
||
def patch_handler_class(tracer, cls, request_hook=None): | ||
def _create_server_histograms(meter) -> Dict[str, Histogram]: | ||
histograms = { | ||
_SERVER_DURATION_HISTOGRAM: meter.create_histogram( | ||
name="http.server.duration", | ||
unit="ms", | ||
description="measures the duration outbound HTTP requests", | ||
), | ||
_SERVER_REQUEST_SIZE_HISTOGRAM: meter.create_histogram( | ||
name="http.server.request.size", | ||
unit="By", | ||
description="measures the size of HTTP request messages (compressed)", | ||
), | ||
_SERVER_RESPONSE_SIZE_HISTOGRAM: meter.create_histogram( | ||
name="http.server.response.size", | ||
unit="By", | ||
description="measures the size of HTTP response messages (compressed)", | ||
), | ||
_SERVER_ACTIVE_REQUESTS_HISTOGRAM: meter.create_up_down_counter( | ||
name="http.server.active_requests", | ||
unit="requests", | ||
description="measures the number of concurrent HTTP requests that are currently in-flight", | ||
), | ||
} | ||
|
||
return histograms | ||
|
||
|
||
def patch_handler_class(tracer, server_histograms, cls, request_hook=None): | ||
if getattr(cls, _OTEL_PATCHED_KEY, False): | ||
return False | ||
|
||
setattr(cls, _OTEL_PATCHED_KEY, True) | ||
_wrap(cls, "prepare", partial(_prepare, tracer, request_hook)) | ||
_wrap(cls, "on_finish", partial(_on_finish, tracer)) | ||
_wrap(cls, "log_exception", partial(_log_exception, tracer)) | ||
_wrap( | ||
cls, | ||
"prepare", | ||
partial(_prepare, tracer, server_histograms, request_hook), | ||
) | ||
_wrap(cls, "on_finish", partial(_on_finish, tracer, server_histograms)) | ||
_wrap( | ||
cls, | ||
"log_exception", | ||
partial(_log_exception, tracer, server_histograms), | ||
) | ||
return True | ||
|
||
|
||
|
@@ -289,28 +362,40 @@ def _wrap(cls, method_name, wrapper): | |
wrapt.apply_patch(cls, method_name, wrapper) | ||
|
||
|
||
def _prepare(tracer, request_hook, func, handler, args, kwargs): | ||
start_time = time_ns() | ||
def _prepare( | ||
tracer, server_histograms, request_hook, func, handler, args, kwargs | ||
): | ||
server_histograms[_START_TIME] = default_timer() | ||
|
||
request = handler.request | ||
if _excluded_urls.url_disabled(request.uri): | ||
return func(*args, **kwargs) | ||
ctx = _start_span(tracer, handler, start_time) | ||
|
||
_record_prepare_metrics(server_histograms, handler) | ||
|
||
ctx = _start_span(tracer, handler) | ||
if request_hook: | ||
request_hook(ctx.span, handler) | ||
return func(*args, **kwargs) | ||
lzchen marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
||
|
||
def _on_finish(tracer, func, handler, args, kwargs): | ||
def _on_finish(tracer, server_histograms, func, handler, args, kwargs): | ||
response = func(*args, **kwargs) | ||
lzchen marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
||
_record_on_finish_metrics(server_histograms, handler) | ||
|
||
_finish_span(tracer, handler) | ||
|
||
return response | ||
|
||
|
||
def _log_exception(tracer, func, handler, args, kwargs): | ||
def _log_exception(tracer, server_histograms, func, handler, args, kwargs): | ||
error = None | ||
if len(args) == 3: | ||
error = args[1] | ||
|
||
_record_on_finish_metrics(server_histograms, handler, error) | ||
|
||
_finish_span(tracer, handler, error) | ||
return func(*args, **kwargs) | ||
|
||
|
@@ -377,11 +462,11 @@ def _get_full_handler_name(handler): | |
return f"{klass.__module__}.{klass.__qualname__}" | ||
|
||
|
||
def _start_span(tracer, handler, start_time) -> _TraceContext: | ||
def _start_span(tracer, handler) -> _TraceContext: | ||
span, token = _start_internal_or_server_span( | ||
tracer=tracer, | ||
span_name=_get_operation_name(handler, handler.request), | ||
start_time=start_time, | ||
start_time=time_ns(), | ||
context_carrier=handler.request.headers, | ||
context_getter=textmap.default_getter, | ||
) | ||
|
@@ -423,7 +508,7 @@ def _finish_span(tracer, handler, error=None): | |
if isinstance(error, tornado.web.HTTPError): | ||
status_code = error.status_code | ||
if not ctx and status_code == 404: | ||
ctx = _start_span(tracer, handler, time_ns()) | ||
ctx = _start_span(tracer, handler) | ||
else: | ||
status_code = 500 | ||
reason = None | ||
|
@@ -462,3 +547,65 @@ def _finish_span(tracer, handler, error=None): | |
if ctx.token: | ||
context.detach(ctx.token) | ||
delattr(handler, _HANDLER_CONTEXT_KEY) | ||
|
||
|
||
def _record_prepare_metrics(server_histograms, handler): | ||
request_size = len(handler.request.body) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Nit: Should be 0 if There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Done |
||
metric_attributes = _create_metric_attributes(handler) | ||
|
||
server_histograms[_SERVER_REQUEST_SIZE_HISTOGRAM].record( | ||
request_size, attributes=metric_attributes | ||
) | ||
|
||
active_requests_attributes = _create_active_requests_attributes( | ||
handler.request | ||
) | ||
server_histograms[_SERVER_ACTIVE_REQUESTS_HISTOGRAM].add( | ||
1, attributes=active_requests_attributes | ||
) | ||
|
||
|
||
def _record_on_finish_metrics(server_histograms, handler, error=None): | ||
elapsed_time = round( | ||
(default_timer() - server_histograms[_START_TIME]) * 1000 | ||
) | ||
|
||
response_size = int(handler._headers.get("Content-Length", 0)) | ||
metric_attributes = _create_metric_attributes(handler) | ||
|
||
if isinstance(error, tornado.web.HTTPError): | ||
metric_attributes[SpanAttributes.HTTP_STATUS_CODE] = error.status_code | ||
|
||
server_histograms[_SERVER_RESPONSE_SIZE_HISTOGRAM].record( | ||
response_size, attributes=metric_attributes | ||
) | ||
|
||
server_histograms[_SERVER_DURATION_HISTOGRAM].record( | ||
elapsed_time, attributes=metric_attributes | ||
) | ||
|
||
active_requests_attributes = _create_active_requests_attributes( | ||
handler.request | ||
) | ||
server_histograms[_SERVER_ACTIVE_REQUESTS_HISTOGRAM].add( | ||
-1, attributes=active_requests_attributes | ||
) | ||
|
||
|
||
def _create_active_requests_attributes(request): | ||
metric_attributes = { | ||
SpanAttributes.HTTP_METHOD: request.method, | ||
SpanAttributes.HTTP_SCHEME: request.protocol, | ||
SpanAttributes.HTTP_FLAVOR: request.version, | ||
SpanAttributes.HTTP_HOST: request.host, | ||
SpanAttributes.HTTP_TARGET: request.path, | ||
} | ||
|
||
return metric_attributes | ||
|
||
|
||
def _create_metric_attributes(handler): | ||
metric_attributes = _create_active_requests_attributes(handler.request) | ||
metric_attributes[SpanAttributes.HTTP_STATUS_CODE] = handler.get_status() | ||
|
||
return metric_attributes |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -14,3 +14,5 @@ | |
|
||
|
||
_instruments = ("tornado >= 5.1.1",) | ||
|
||
_supports_metrics = True |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please move this to Unreleased section
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Like this?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This was part of prev release changelog prior. I mean add the entry under #Added of Unreleased section
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ohh I got it!
Fixed