-
Notifications
You must be signed in to change notification settings - Fork 651
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
Added ability to extract span attributes from tornado request objects #1178
Changes from 4 commits
17cc913
7ef87f9
1f4fd30
5c0bc91
ea81967
dd22bd8
20e08ab
7737b36
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 | ||||
---|---|---|---|---|---|---|
|
@@ -50,6 +50,7 @@ def get(self): | |||||
from opentelemetry.instrumentation.instrumentor import BaseInstrumentor | ||||||
from opentelemetry.instrumentation.tornado.version import __version__ | ||||||
from opentelemetry.instrumentation.utils import ( | ||||||
extract_attributes_from_object, | ||||||
http_status_to_canonical_code, | ||||||
unwrap, | ||||||
) | ||||||
|
@@ -71,7 +72,17 @@ def get_excluded_urls(): | |||||
return ExcludeList(urls) | ||||||
|
||||||
|
||||||
def get_traced_request_attrs(): | ||||||
attrs = configuration.Configuration().TORNADO_TRACED_REQUEST_ATTRS or "" | ||||||
if attrs: | ||||||
attrs = [attr.strip() for attr in attrs.split(",")] | ||||||
else: | ||||||
attrs = [] | ||||||
return attrs | ||||||
|
||||||
|
||||||
_excluded_urls = get_excluded_urls() | ||||||
_traced_attrs = get_traced_request_attrs() | ||||||
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. What about this to not have the function above?
Suggested change
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. Very similar functionality above uses a function so I think it's better to be consistent with existing code. |
||||||
|
||||||
|
||||||
class TornadoInstrumentor(BaseInstrumentor): | ||||||
|
@@ -196,7 +207,7 @@ def _get_attributes_from_request(request): | |||||
if request.remote_ip: | ||||||
attrs["net.peer.ip"] = request.remote_ip | ||||||
|
||||||
return attrs | ||||||
return extract_attributes_from_object(request, _traced_attrs, attrs) | ||||||
|
||||||
|
||||||
def _get_operation_name(handler, request): | ||||||
|
@@ -211,6 +222,7 @@ def _start_span(tracer, handler, start_time) -> _TraceContext: | |||||
_get_header_from_request_headers, handler.request.headers, | ||||||
) | ||||||
) | ||||||
|
||||||
span = tracer.start_span( | ||||||
_get_operation_name(handler, handler.request), | ||||||
kind=trace.SpanKind.SERVER, | ||||||
|
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.
is it worth moving this code into the utils package as well since it seems likely to be repeated?
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.
Agree. I think it might be better to have convenience methods on the configuration class itself for common config items. Something like,
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.
Created an issue for it. #1217