Tornado attributes #706
Tornado attributes #706
Conversation
Tornado sets remote_ip to the value of X-Forwarded-For or X-Real-IP if the 'xheaders' setting is set.
The spec has a 'http.route' which unfortunately seems difficult to get at. However the full class name is readily available and most helpful.
| @@ -252,6 +249,11 @@ def _get_operation_name(handler, request): | |||
| return f"{class_name}.{request.method.lower()}" | |||
|
|
|||
|
|
|||
| def _get_full_handler_name(handler): | |||
| klass = type(handler) | |||
| return f"{klass.__module__}.{klass.__qualname__}" | |||
There was a problem hiding this comment.
would this work fine for dynamically generated handlers within a function? I know that is not very common but sounds like an edge case. Any concerns regarding that?
There was a problem hiding this comment.
I believe __qualname__ would do the right thing with closures, see the examples in PEP 3155
that is OK as long as we do it defensively |
|
I added reading Off-topic: I see the hacktoberfest topic is set on some open-telemetry repos but not here, I'm not sure if it's on purpose 😉 |
|
Last time there was a "fest" we had a few dozen people open PRs to add new lines in markdown files to earn some free merch 😂 is this similar? |
.../opentelemetry-instrumentation-tornado/src/opentelemetry/instrumentation/tornado/__init__.py
Outdated
Show resolved
Hide resolved
| if request.remote_ip: | ||
| attrs[SpanAttributes.NET_PEER_IP] = request.remote_ip |
There was a problem hiding this comment.
Please add a comment here for future contributors describing why we assign these to different attributes.
|
Done! hacktoberfest is usually ok in my experience, last year really did not go well. I believe they have made some changes to the procedure this year but I can't vouch for the masses. |
.../opentelemetry-instrumentation-tornado/src/opentelemetry/instrumentation/tornado/__init__.py
Outdated
Show resolved
Hide resolved
.../opentelemetry-instrumentation-tornado/src/opentelemetry/instrumentation/tornado/__init__.py
Outdated
Show resolved
Hide resolved
- Don't set NET_PEER_IP if '_orig_remote_ip' is not set - Add a comment about the difference between the attributes
|
(this should take care of test failures) |
Description
http.client_ipinstead ofnet.peer.ip: the value is changed by Tornado toX-Forwarded-FororX-Real-IP, see Tornado code, see specnet.peer.ipis in_orig_remote_ip, however I thought it best not to read a private field. Let me know if it's ok.handlerto the fully-qualified name of the request handler classhttp.route, only the base handler class's name as the span name. I didn't find an easy way to read the route pattern, the name of the handler seems like the next best thingType of change
Please delete options that are not relevant.
How Has This Been Tested?
tox -e py38-test-instrumentation-tornado(TestTornadoInstrumentationupdated)Does This PR Require a Core Repo Change?
Checklist:
See contributing.md for styleguide, changelog guidelines, and more.