Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
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
31 changes: 31 additions & 0 deletions sentry_sdk/consts.py
Original file line number Diff line number Diff line change
Expand Up @@ -404,6 +404,12 @@ class SPANDATA:
Example: template.cache.some_item.867da7e2af8e6b2f3aa7213a4080edb3
"""

CLIENT_ADDRESS = "client.address"
"""
Client address of the network connection - IP address or Unix domain socket name.
Example: "10.1.2.80"
"""

CODE_FILEPATH = "code.filepath"
"""
.. deprecated::
Expand Down Expand Up @@ -819,12 +825,25 @@ class SPANDATA:
Example: GET
"""

HTTP_REQUEST_HEADER = "http.request.header"
"""
Prefix for HTTP request header attributes. The header name (lowercased) is
appended to form the full attribute key.
Example: "http.request.header.content-type"
"""

HTTP_REQUEST_METHOD = "http.request.method"
"""
The HTTP method used.
Example: GET
"""

HTTP_REQUEST_BODY_DATA = "http.request.body.data"
"""
The HTTP request body data as string.
Example: "[{\"role\": \"user\", \"message\": \"hello\"}]"
"""

HTTP_QUERY = "http.query"
"""
The Query string present in the URL.
Expand Down Expand Up @@ -863,6 +882,12 @@ class SPANDATA:
The messaging system's name, e.g. `kafka`, `aws_sqs`
"""

NETWORK_PROTOCOL_NAME = "network.protocol.name"
"""
The application layer protocol name used for the network connection.
Example: "http", "https"
"""

NETWORK_PEER_ADDRESS = "network.peer.address"
"""
Peer address of the network connection - IP address or Unix domain socket name.
Expand Down Expand Up @@ -942,6 +967,12 @@ class SPANDATA:
Example: "MainThread"
"""

USER_IP_ADDRESS = "user.ip_address"
"""
The IP address of the user that triggered the request.
Example: "10.1.2.80"
"""

URL_FULL = "url.full"
"""
The URL of the resource that was fetched.
Expand Down
134 changes: 116 additions & 18 deletions sentry_sdk/integrations/tornado.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,19 +4,23 @@

import sentry_sdk
from sentry_sdk.api import continue_trace
from sentry_sdk.consts import OP
from sentry_sdk.consts import OP, SPANDATA
from sentry_sdk.integrations import DidNotEnable, Integration, _check_minimum_version
from sentry_sdk.integrations._wsgi_common import (
RequestExtractor,
_filter_headers,
_is_json_content_type,
request_body_within_bounds,
)
from sentry_sdk.integrations.logging import ignore_logger
from sentry_sdk.scope import should_send_default_pii
from sentry_sdk.traces import NoOpStreamedSpan, SegmentSource, StreamedSpan
from sentry_sdk.tracing import TransactionSource
from sentry_sdk.tracing_utils import has_span_streaming_enabled
from sentry_sdk.utils import (
CONTEXTVARS_ERROR_MESSAGE,
HAS_REAL_CONTEXTVARS,
AnnotatedValue,
capture_internal_exceptions,
ensure_integration_enabled,
event_from_exception,
Expand All @@ -33,9 +37,10 @@
from typing import TYPE_CHECKING

if TYPE_CHECKING:
from typing import Any, Callable, Dict, Generator, Optional
from typing import Any, Callable, ContextManager, Dict, Generator, Optional, Union

from sentry_sdk._types import Event, EventProcessor
from sentry_sdk.tracing import Span


class TornadoIntegration(Integration):
Expand Down Expand Up @@ -97,6 +102,9 @@ def sentry_log_exception(
RequestHandler.log_exception = sentry_log_exception


_DEFAULT_TRANSACTION_NAME = "generic Tornado request"


@contextlib.contextmanager
def _handle_request_impl(self: "RequestHandler") -> "Generator[None, None, None]":
integration = sentry_sdk.get_client().get_integration(TornadoIntegration)
Expand All @@ -106,6 +114,8 @@ def _handle_request_impl(self: "RequestHandler") -> "Generator[None, None, None]
return

weak_handler = weakref.ref(self)
client = sentry_sdk.get_client()
is_span_streaming_enabled = has_span_streaming_enabled(client.options)

with sentry_sdk.isolation_scope() as scope:
headers = self.request.headers
Expand All @@ -114,22 +124,110 @@ def _handle_request_impl(self: "RequestHandler") -> "Generator[None, None, None]
processor = _make_event_processor(weak_handler)
scope.add_event_processor(processor)

transaction = continue_trace(
headers,
op=OP.HTTP_SERVER,
# Like with all other integrations, this is our
# fallback transaction in case there is no route.
# sentry_urldispatcher_resolve is responsible for
# setting a transaction name later.
name="generic Tornado request",
source=TransactionSource.ROUTE,
origin=TornadoIntegration.origin,
)

with sentry_sdk.start_transaction(
transaction, custom_sampling_context={"tornado_request": self.request}
):
yield
span_ctx: "ContextManager[Union[Span, StreamedSpan, None]]"

if is_span_streaming_enabled:
sentry_sdk.traces.continue_trace(dict(headers))
scope.set_custom_sampling_context({"tornado_request": self.request})
Comment thread
sl0thentr0py marked this conversation as resolved.

span_ctx = sentry_sdk.traces.start_span(
name=_DEFAULT_TRANSACTION_NAME,
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

_DEFAULT_TRANSACTION_NAME is a bit strange to read in the context of streamed spans where there is no longer a "transaction".

I'm ok with either leaving this as-is until we remove the transaction v1 spans and updating this then, or doing a rename that indicates that this name is the default for a "root" span.

attributes={
"sentry.op": OP.HTTP_SERVER,
"sentry.origin": TornadoIntegration.origin,
"sentry.span.source": SegmentSource.ROUTE,
},
parent_span=None,
)
Comment thread
sl0thentr0py marked this conversation as resolved.
Comment thread
sl0thentr0py marked this conversation as resolved.
else:
transaction = continue_trace(
headers,
op=OP.HTTP_SERVER,
# Like with all other integrations, this is our
# fallback transaction in case there is no route.
# sentry_urldispatcher_resolve is responsible for
# setting a transaction name later.
name=_DEFAULT_TRANSACTION_NAME,
source=TransactionSource.ROUTE,
origin=TornadoIntegration.origin,
)
span_ctx = sentry_sdk.start_transaction(
transaction,
custom_sampling_context={"tornado_request": self.request},
)

with span_ctx as span:
try:
yield
finally:
if isinstance(span, StreamedSpan) and not isinstance(
span, NoOpStreamedSpan
):
Comment on lines +163 to +165
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This can be made a bit more concise with type(span) is StreamedSpan:

Suggested change
if isinstance(span, StreamedSpan) and not isinstance(
span, NoOpStreamedSpan
):
if type(span) is StreamedSpan:

with capture_internal_exceptions():
for attr, value in _get_request_attributes(
self.request
).items():
span.set_attribute(attr, value)

with capture_internal_exceptions():
method = getattr(self, self.request.method.lower(), None)
if method is not None:
span_name = transaction_from_function(method)
if span_name:
span.name = span_name
span.set_attribute(
"sentry.span.source",
SegmentSource.COMPONENT,
Comment on lines +178 to +180
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Is there a reason why setting this attribute is being gated by whether or not a span_name exists? This looks like something that we could set regardless.

)

with capture_internal_exceptions():
status_int = self.get_status()
span.set_attribute(SPANDATA.HTTP_STATUS_CODE, status_int)
span.status = "error" if status_int >= 400 else "ok"
Comment thread
cursor[bot] marked this conversation as resolved.
Comment thread
sl0thentr0py marked this conversation as resolved.


def _get_request_attributes(request: "Any") -> "Dict[str, Any]":
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Are we still capturing the request body in the streaming path? See https://getsentry.github.io/sentry-conventions/attributes/http/#http-request-body-data

For prior art IIRC @ericapisani did this for another integration, not sure which exactly

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

starlette and aiohttp were the couple that I worked on that had this

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

added req body

attributes = {} # type: Dict[str, Any]

if request.method:
attributes[SPANDATA.HTTP_REQUEST_METHOD] = request.method.upper()

headers = _filter_headers(dict(request.headers), use_annotated_value=False)
for header, value in headers.items():
attributes[f"{SPANDATA.HTTP_REQUEST_HEADER}.{header.lower()}"] = value

if request.query:
attributes[SPANDATA.URL_QUERY] = request.query

attributes[SPANDATA.URL_FULL] = request.full_url()

if request.protocol:
attributes[SPANDATA.NETWORK_PROTOCOL_NAME] = request.protocol

if should_send_default_pii() and request.remote_ip:
attributes[SPANDATA.CLIENT_ADDRESS] = request.remote_ip
attributes[SPANDATA.USER_IP_ADDRESS] = request.remote_ip

with capture_internal_exceptions():
raw_data = _get_tornado_request_data(request)
body_data = raw_data.value if isinstance(raw_data, AnnotatedValue) else raw_data
if body_data is not None:
attributes[SPANDATA.HTTP_REQUEST_BODY_DATA] = body_data

return attributes


def _get_tornado_request_data(
request: "Any",
) -> "Union[Optional[str], AnnotatedValue]":
body = request.body
if not body:
return None

if not request_body_within_bounds(sentry_sdk.get_client(), len(body)):
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Is the body at this point in time the "realized" result? For example, the result of request.json()?

I'm also wondering if using the content-length header - does tornado set this automatically for incoming requests?

return AnnotatedValue.substituted_because_over_size_limit()

return body.decode("utf-8", "replace")
Comment thread
sl0thentr0py marked this conversation as resolved.


@ensure_integration_enabled(TornadoIntegration)
Expand Down
Loading
Loading