diff --git a/sentry_sdk/consts.py b/sentry_sdk/consts.py index 90e89afc95..5b037a66d0 100644 --- a/sentry_sdk/consts.py +++ b/sentry_sdk/consts.py @@ -1214,6 +1214,12 @@ class SPANDATA: Used in inbound filters. """ + SENTRY_KIND = "sentry.kind" + """ + Used to clarify the relationship between parents and children, or to distinguish between spans, e.g. a `server` and `client` span with the same name. + Example: "client", "server", "producer", "consumer", "internal" + """ + SENTRY_OP = "sentry.op" """ The operation of a span. diff --git a/sentry_sdk/integrations/boto3/_client.py b/sentry_sdk/integrations/boto3/_client.py index d30770ebe7..32015c802e 100644 --- a/sentry_sdk/integrations/boto3/_client.py +++ b/sentry_sdk/integrations/boto3/_client.py @@ -14,17 +14,22 @@ _set_span_attributes, _start_client_span, ) +from sentry_sdk.integrations.boto3._services.registry import ( + _resolve_service, +) from sentry_sdk.integrations.boto3.consts import IDENTIFIER from sentry_sdk.traces import NoOpStreamedSpan, StreamedSpan from sentry_sdk.utils import capture_internal_exceptions if TYPE_CHECKING: - from typing import Any, Iterator, Optional, Union + from typing import Any, Dict, Iterator, Optional, Union + from sentry_sdk._types import Attributes from sentry_sdk.tracing import Span try: from botocore.client import BaseClient + from botocore.exceptions import ClientError except ImportError: raise DidNotEnable("botocore not installed") @@ -34,12 +39,12 @@ def _activate_client_span( span: "Union[Span, StreamedSpan]", ) -> "Iterator[Union[Span, StreamedSpan]]": """ - Activate the boto span temporarily during `_make_api_call()` without ending it. + Activate the client span temporarily during `_make_api_call()` without ending it. Botocore returns a `StreamingBody` before its bytes are consumed. Using the context manager would finish it as soon as `_make_api_call()` returns, so restore the caller's span here and let the `StreamingBody` wrapper finish - the boto span when body is consumed/closed. + the client span when the body is consumed or closed. faulty: desired: boto3 [_make_api_call] boto3 [_make_api_call------] @@ -74,13 +79,13 @@ def _patch_botocore_client() -> None: def sentry_patched_init(self: "BaseClient", *args: "Any", **kwargs: "Any") -> None: orig_init(self, *args, **kwargs) - meta = self.meta - meta.events.register("request-created", _sentry_request_created) - # run after other `before-sign` handlers so existing baggage is preserved. - meta.events.register_last("before-sign", _sentry_before_sign) + with capture_internal_exceptions(): + self.meta.events.register("request-created", _sentry_request_created) + # run after other `before-sign` handlers so existing baggage is preserved. + self.meta.events.register_last("before-sign", _sentry_before_sign) def sentry_patched_make_api_call( - self: "BaseClient", operation_name: str, api_params: "Any" + self: "BaseClient", operation_name: str, api_params: "Dict[str, Any]" ) -> "Any": """ Track a single API call, including retries, serialization, and endpoint @@ -93,15 +98,14 @@ def sentry_patched_make_api_call( if client.get_integration(IDENTIFIER) is None: return orig_make_api_call(self, operation_name, api_params) - ctx = AwsCallContext(operation_name, api_params) - - # add optional metadata to context. - with capture_internal_exceptions(): - ctx.add_metadata(self) - span: "Optional[Union[Span, StreamedSpan]]" = None with capture_internal_exceptions(): - span = _start_client_span(ctx) + ctx = AwsCallContext(operation_name, api_params) + with capture_internal_exceptions(): + # add optional metadata to the context, e.g. service-name, region-name, etc. + ctx.add_metadata(self) + service_ext = _resolve_service(ctx.service_name) + span = _start_client_span(ctx, service_ext) if span is None: return orig_make_api_call(self, operation_name, api_params) @@ -109,16 +113,31 @@ def sentry_patched_make_api_call( # activate without finishing; a streaming response may outlive the call. span_ctx = _activate_client_span(span) + attributes: "Attributes" = {} try: with span_ctx: try: parsed = orig_make_api_call(self, operation_name, api_params) except BaseException as error: + if service_ext is not None and isinstance(error, ClientError): + with capture_internal_exceptions(): + attributes.update( + service_ext.get_response_attributes(ctx, error.response) + ) + with capture_internal_exceptions(): + attributes.update(_get_error_attributes(error)) with capture_internal_exceptions(): - _set_span_attributes(span, _get_error_attributes(error)) + _set_span_attributes(span, attributes) raise + if service_ext is not None: + with capture_internal_exceptions(): + attributes.update( + service_ext.get_response_attributes(ctx, parsed) + ) + with capture_internal_exceptions(): + attributes.update(_get_response_attributes(parsed)) with capture_internal_exceptions(): - _set_span_attributes(span, _get_response_attributes(parsed)) + _set_span_attributes(span, attributes) except BaseException as error: _finish_span(span, error) raise diff --git a/sentry_sdk/integrations/boto3/_context.py b/sentry_sdk/integrations/boto3/_context.py index 83867d20b4..4ab95eb366 100644 --- a/sentry_sdk/integrations/boto3/_context.py +++ b/sentry_sdk/integrations/boto3/_context.py @@ -23,40 +23,27 @@ class AwsCallContext: "params", ) - def __init__(self, operation_name: str, params: "Any") -> None: - self.operation_name: str = operation_name - self.params: "Dict[str, Any]" = {} + def __init__(self, operation_name: str, params: "Dict[str, Any]") -> None: + self.operation_name: "str" = operation_name + self.params: "Dict[str, Any]" = dict(params) self.service_name: "Optional[str]" = None self.service_id: "Optional[str]" = None self.service_id_hyphenized: "Optional[str]" = None self.region_name: "Optional[str]" = None self.endpoint_url: "Optional[str]" = None - if isinstance(params, dict): - with capture_internal_exceptions(): - self.params = dict(params) - def add_metadata(self, client: "BaseClient") -> None: - def _get_attr(obj: "Any", name: str) -> "Any": - if obj is None: - return None - - with capture_internal_exceptions(): - return getattr(obj, name) - - client_meta = _get_attr(client, "meta") - service_model = _get_attr(client_meta, "service_model") - - # botocore's internal identifier, e.g. `apigateway`. - self.service_name = _get_attr(service_model, "service_name") - - # modeled AWS service identity used in span names, e.g. `API Gateway`. - service_id = _get_attr(service_model, "service_id") - if service_id is not None: - with capture_internal_exceptions(): - self.service_id = str(service_id) - with capture_internal_exceptions(): - self.service_id_hyphenized = service_id.hyphenize() - - self.region_name = _get_attr(client_meta, "region_name") - self.endpoint_url = _get_attr(client_meta, "endpoint_url") + with capture_internal_exceptions(): + service_model = client.meta.service_model + # botocore's internal identifier, e.g. `apigateway`. + self.service_name = service_model.service_name + service_id = service_model.service_id + # modeled AWS service identity used in span names, e.g. `API Gateway`. + self.service_id = str(service_id) + self.service_id_hyphenized = service_id.hyphenize() + + with capture_internal_exceptions(): + self.region_name = client.meta.region_name + + with capture_internal_exceptions(): + self.endpoint_url = client.meta.endpoint_url diff --git a/sentry_sdk/integrations/boto3/_instrumentation.py b/sentry_sdk/integrations/boto3/_instrumentation.py index 74f8cce948..3c180a025c 100644 --- a/sentry_sdk/integrations/boto3/_instrumentation.py +++ b/sentry_sdk/integrations/boto3/_instrumentation.py @@ -1,4 +1,4 @@ -from typing import TYPE_CHECKING +from typing import TYPE_CHECKING, cast from urllib.parse import urlsplit import sentry_sdk @@ -29,6 +29,7 @@ from sentry_sdk._types import Attributes from sentry_sdk.integrations.boto3._context import AwsCallContext + from sentry_sdk.integrations.boto3._services.base import _ServiceExtension try: from botocore.awsrequest import AWSRequest @@ -82,7 +83,7 @@ def _get_client_attributes( attributes: "Attributes" = {} # `rpc.service` is deprecated in OTel, but js still uses it. - if ctx.service_id: + if ctx.service_id is not None: attributes[SPANDATA.RPC_SERVICE] = ctx.service_id if ctx.region_name: @@ -99,7 +100,7 @@ def _get_response_attributes(response: "Mapping[str, Any]") -> "Attributes": # botocore injects HTTP status into `ResponseMetadata` after parsing. # https://github.com/boto/botocore/blob/358f8eec8c76201bb1a7a35644abcbc9036de7ed/botocore/parsers.py#L273-L284 status_code = metadata.get("HTTPStatusCode") - if isinstance(status_code, int) and 100 <= status_code <= 599: + if status_code is not None: attributes[SPANDATA.HTTP_STATUS_CODE] = status_code retry_attempts = metadata.get("RetryAttempts", 0) @@ -108,33 +109,19 @@ def _get_response_attributes(response: "Mapping[str, Any]") -> "Attributes": headers = metadata.get("HTTPHeaders", {}) - request_id = next( - ( - value - for value in ( - metadata.get("RequestId"), - headers.get("x-amzn-requestid"), - headers.get("x-amzn-request-id"), - headers.get("x-amz-request-id"), - ) - if isinstance(value, str) and value - ), - None, + request_id = ( + metadata.get("RequestId") + or headers.get("x-amzn-requestid") + or headers.get("x-amzn-request-id") + or headers.get("x-amz-request-id") ) - if request_id is not None: + if request_id: attributes[SPANDATA.AWS_REQUEST_ID] = request_id # S3's `HostId` is the extended request ID returned in `x-amz-id-2`. # https://docs.aws.amazon.com/AmazonS3/latest/developerguide/get-request-ids.html - extended_request_id = next( - ( - value - for value in (metadata.get("HostId"), headers.get("x-amz-id-2")) - if isinstance(value, str) and value - ), - None, - ) - if extended_request_id is not None: + extended_request_id = metadata.get("HostId") or headers.get("x-amz-id-2") + if extended_request_id: attributes[SPANDATA.AWS_EXTENDED_REQUEST_ID] = extended_request_id return attributes @@ -170,25 +157,44 @@ def _get_error_attributes(exception: "BaseException") -> "Attributes": def _start_client_span( ctx: "AwsCallContext", + service_ext: "Optional[_ServiceExtension]" = None, ) -> "Optional[Union[Span, StreamedSpan]]": client = sentry_sdk.get_client() - if client.get_integration(IDENTIFIER) is None: - return None - # use unknown if `service_id_hyphenized` is not set so span name can still be created. + # use "unknown" if `service_id_hyphenized` is not set so span name can still be created. # e.g. "aws.unknown.GetObject" service_name = ctx.service_id_hyphenized or "unknown" span_name = f"aws.{service_name}.{ctx.operation_name}" attributes: "Attributes" = { SPANDATA.RPC_METHOD: ctx.operation_name, SPANDATA.RPC_SYSTEM_NAME: AWS_RPC_SYSTEM_NAME, + # all client call spans are by default "client" spans. + # https://opentelemetry.io/docs/specs/semconv/cloud-providers/aws-sdk/#aws-sdk-spans + SPANDATA.SENTRY_KIND: "client", } with capture_internal_exceptions(): attributes.update(_get_client_attributes(ctx)) + + # `sentry.span_op` and `sentry.span_origin` are set to generic defaults; + # a service extension can override them with `get_span_op()` and `get_span_origin()`. span_op = OP.HTTP_CLIENT span_origin = ORIGIN + if service_ext is not None: + with capture_internal_exceptions(): + service_op = service_ext.get_span_op(ctx) + if service_op is not None: + span_op = service_op + + with capture_internal_exceptions(): + service_origin = service_ext.get_span_origin(ctx) + if service_origin is not None: + span_origin = service_origin + + with capture_internal_exceptions(): + attributes.update(service_ext.get_request_attributes(ctx)) + if has_span_streaming_enabled(client.options): if sentry_sdk.traces.get_current_span() is None: return None @@ -217,7 +223,7 @@ def _start_client_span( with capture_internal_exceptions(): _set_span_attributes(span, attributes) with capture_internal_exceptions(): - if ctx.service_id_hyphenized: + if ctx.service_id_hyphenized is not None: span.set_tag("aws.service_id", ctx.service_id_hyphenized) span.set_tag("aws.operation_name", ctx.operation_name) return span @@ -254,10 +260,11 @@ def _instrument_streaming_body( if isinstance(span, StreamedSpan): streaming_span = sentry_sdk.traces.start_span( name=span.name, - # keep stream span under the boto span after `_make_api_call()` returns. + # keep the stream span under the client span after + # `_make_api_call()` returns. parent_span=span, # the body may outlive the api call, so keep it inactive. Otherwise it - # 1. could restore the already-finished boto span when it ends; 2. make + # 1. could restore the already-finished client span when it ends; 2. make # unrelated new spans attach to the stream span since it's the current span. active=False, attributes={ @@ -281,7 +288,8 @@ def finish_span(error: "Optional[BaseException]" = None) -> None: return finished = True - # finish stream span before boto span, and only once across read/close. + # finish the stream span before the client span, and only once across + # read and close. if error is not None: with capture_internal_exceptions(): attributes = _get_error_attributes(error) @@ -292,13 +300,8 @@ def finish_span(error: "Optional[BaseException]" = None) -> None: _finish_span(span, error) def content_length_reached() -> bool: - content_length = getattr(body, "_content_length", None) - amount_read = getattr(body, "_amount_read", None) - return ( - content_length is not None - and amount_read is not None - and amount_read >= int(content_length) - ) + content_length = body._content_length # type: ignore[attr-defined] + return content_length is not None and body._amount_read >= int(content_length) # type: ignore[attr-defined] def sentry_streaming_body_read(*args: "Any", **kwargs: "Any") -> bytes: nonlocal read_in_progress @@ -362,17 +365,15 @@ def _set_request_attributes( client = sentry_sdk.get_client() parsed_url = None - if request.url is not None: - with capture_internal_exceptions(): - parsed_url = parse_url(request.url, sanitize=False) + with capture_internal_exceptions(): + parsed_url = parse_url(cast(str, request.url), sanitize=False) # overwrite server attributes when actual request URL is resolved. _set_span_attributes(span, _get_server_attributes(request.url)) if isinstance(span, StreamedSpan): span.set_attributes(get_url_attributes(client, parsed_url)) - if request.method is not None: - span.set_attribute(SPANDATA.HTTP_REQUEST_METHOD, request.method) + span.set_attribute(SPANDATA.HTTP_REQUEST_METHOD, cast(str, request.method)) return if parsed_url is not None: @@ -380,24 +381,21 @@ def _set_request_attributes( span.set_data(SPANDATA.HTTP_QUERY, parsed_url.query) span.set_data(SPANDATA.HTTP_FRAGMENT, parsed_url.fragment) - if request.method is not None: - span.set_data(SPANDATA.HTTP_METHOD, request.method) + span.set_data(SPANDATA.HTTP_METHOD, request.method) def _add_request_breadcrumb(request: "AWSRequest") -> None: client = sentry_sdk.get_client() parsed_url = None - if request.url is not None: - with capture_internal_exceptions(): - parsed_url = parse_url(request.url, sanitize=False) + with capture_internal_exceptions(): + parsed_url = parse_url(cast(str, request.url), sanitize=False) breadcrumb: "dict[str, Any]" = {} if has_span_streaming_enabled(client.options): breadcrumb.update(get_url_attributes(client, parsed_url)) - if request.method is not None: - breadcrumb[SPANDATA.HTTP_REQUEST_METHOD] = request.method + breadcrumb[SPANDATA.HTTP_REQUEST_METHOD] = request.method else: if parsed_url is not None: breadcrumb.update( @@ -408,8 +406,7 @@ def _add_request_breadcrumb(request: "AWSRequest") -> None: } ) - if request.method is not None: - breadcrumb[SPANDATA.HTTP_METHOD] = request.method + breadcrumb[SPANDATA.HTTP_METHOD] = request.method add_http_breadcrumb(None, breadcrumb) @@ -438,10 +435,9 @@ def _sentry_request_created( if span is None: return - # an ignored streamed span is not activated; avoid enriching its parent. - if isinstance(span, StreamedSpan): - if not (span.get_attributes().get(SPANDATA.SENTRY_ORIGIN) == ORIGIN): - return + # an ignored streamed span is not active; avoid enriching its parent. + if isinstance(span, StreamedSpan) and span.active: + return _set_request_attributes(span, request) # each attempt has a fresh `request.context`; carry the active client span. diff --git a/sentry_sdk/integrations/boto3/_services/__init__.py b/sentry_sdk/integrations/boto3/_services/__init__.py new file mode 100644 index 0000000000..e69de29bb2 diff --git a/sentry_sdk/integrations/boto3/_services/base.py b/sentry_sdk/integrations/boto3/_services/base.py new file mode 100644 index 0000000000..0d9cb1c24d --- /dev/null +++ b/sentry_sdk/integrations/boto3/_services/base.py @@ -0,0 +1,36 @@ +from typing import TYPE_CHECKING + +if TYPE_CHECKING: + from typing import Any, Optional + + from sentry_sdk._types import Attributes + from sentry_sdk.integrations.boto3._context import AwsCallContext + + +class _ServiceExtension: + """ + Optional hooks for adding service-specific behavior to AWS client + span; non-overridden methods keep the generic instrumentation. + Services without a registered extension in `_SERVICE_EXTENSIONS` continue + to use the generic instrumentation. + """ + + __slots__ = () + + def get_span_op(self, ctx: "AwsCallContext") -> "Optional[str]": + """Return an optional `sentry.op` override, or `None` to keep the default.""" + return None + + def get_span_origin(self, ctx: "AwsCallContext") -> "Optional[str]": + """Return an optional `sentry.origin` override, or `None` to keep the default.""" + return None + + def get_request_attributes(self, ctx: "AwsCallContext") -> "Attributes": + """Return request attributes to add before the AWS request is made.""" + return {} + + def get_response_attributes( + self, ctx: "AwsCallContext", response: "Any" + ) -> "Attributes": + """Return response attributes to add after the AWS request is made.""" + return {} diff --git a/sentry_sdk/integrations/boto3/_services/registry.py b/sentry_sdk/integrations/boto3/_services/registry.py new file mode 100644 index 0000000000..b387db0f26 --- /dev/null +++ b/sentry_sdk/integrations/boto3/_services/registry.py @@ -0,0 +1,22 @@ +from typing import TYPE_CHECKING + +if TYPE_CHECKING: + from typing import Dict, Optional + + from sentry_sdk.integrations.boto3._services.base import _ServiceExtension + + +# add a ServiceExtension here when one is implemented. for example: +# _SERVICE_EXTENSIONS = {"s3": _S3Extension()} +# when py 3.15 drops, we might want to take a look at using +# a lazy-loading approach using the new `lazy` keyword. +_SERVICE_EXTENSIONS: "Dict[str, _ServiceExtension]" = {} + + +def _resolve_service( + service_name: "Optional[str]", +) -> "Optional[_ServiceExtension]": + """Return the extension for a service, or `None` for generic instrumentation.""" + if service_name is None: + return None + return _SERVICE_EXTENSIONS.get(service_name) diff --git a/tests/integrations/boto3/test_client.py b/tests/integrations/boto3/test_client.py index 9b2d502115..c309549316 100644 --- a/tests/integrations/boto3/test_client.py +++ b/tests/integrations/boto3/test_client.py @@ -12,15 +12,13 @@ import sentry_sdk from sentry_sdk.consts import OP, SPANDATA from sentry_sdk.integrations.boto3 import Boto3Integration -from sentry_sdk.integrations.boto3._instrumentation import ( - _get_response_attributes, - _instrument_streaming_body, -) +from sentry_sdk.integrations.boto3._services.base import _ServiceExtension +from sentry_sdk.integrations.boto3._services.registry import _SERVICE_EXTENSIONS from sentry_sdk.integrations.boto3.consts import AWS_RPC_SYSTEM_NAME, ORIGIN from sentry_sdk.integrations.stdlib import StdlibIntegration from sentry_sdk.traces import StreamedSpan from sentry_sdk.tracing import Span -from tests.integrations.boto3.aws_mock import Body +from tests.integrations.boto3.aws_mock import Body, MockResponse session = boto3.Session( # type: ignore[attr-defined] aws_access_key_id="-", @@ -188,57 +186,6 @@ def record_client_span(request, **kwargs): assert span[end_timestamp] is not None -@pytest.mark.parametrize("span_streaming", [True, False]) -def test_streaming_body_instrumentation_setup_failure_finishes_stream_span( - sentry_init, - capture_items, - span_streaming, -): - sentry_init( - traces_sample_rate=1.0, - trace_lifecycle="stream" if span_streaming else "static", - integrations=[Boto3Integration()], - server_name="", - ) - - class _RawStreamLookupFailingBody(StreamingBody): - @property - def _raw_stream(self): - raise RuntimeError("raw stream lookup failed") - - @_raw_stream.setter - def _raw_stream(self, raw_stream): - self._raw_stream_value = raw_stream - - body = _RawStreamLookupFailingBody(Body(b"x"), "1") - - def invoke(): - if not span_streaming: - with sentry_sdk.start_span( - name="client", op=OP.HTTP_CLIENT, origin=ORIGIN - ) as span: - with pytest.raises(RuntimeError, match="raw stream lookup failed"): - _instrument_streaming_body(span, {"Body": body}) - return - - span = sentry_sdk.traces.start_span( # type: ignore[attr-defined] - name="client", - attributes={ - SPANDATA.SENTRY_OP: OP.HTTP_CLIENT, - SPANDATA.SENTRY_ORIGIN: ORIGIN, - }, - active=False, - ) - with pytest.raises(RuntimeError, match="raw stream lookup failed"): - _instrument_streaming_body(span, {"Body": body}) - - spans_by_op = _capture_boto3_spans_by_op(invoke, capture_items, span_streaming) - stream_spans = spans_by_op.get(OP.HTTP_CLIENT_STREAM, []) - - assert len(stream_spans) == 1 - _assert_span_finished(stream_spans[0], span_streaming) - - def test_non_body_stream_does_not_delay_client_span(sentry_init, capture_items): sentry_init( traces_sample_rate=1.0, @@ -320,7 +267,12 @@ def respond(request, **kwargs): return request_span_ids -def _capture_boto3_spans_by_op(invoke_client_method, capture_items, span_streaming): +def _capture_boto3_spans_by_op( + invoke_client_method, + capture_items, + span_streaming, + expected_origin=ORIGIN, +): items = capture_items() if span_streaming: @@ -332,14 +284,17 @@ def _capture_boto3_spans_by_op(invoke_client_method, capture_items, span_streami item.payload for item in items if item.type == "span" - and item.payload["attributes"].get(SPANDATA.SENTRY_ORIGIN) == ORIGIN + and item.payload["attributes"].get(SPANDATA.SENTRY_ORIGIN) + == expected_origin ] else: with sentry_sdk.start_transaction(): invoke_client_method() transaction = next(item.payload for item in items if item.type == "transaction") - spans = [span for span in transaction["spans"] if span["origin"] == ORIGIN] + spans = [ + span for span in transaction["spans"] if span["origin"] == expected_origin + ] spans_by_op = {} for span in spans: @@ -389,97 +344,69 @@ def _span_attributes(span, span_streaming): return span["attributes"] if span_streaming else span["data"] -@pytest.mark.parametrize( - ("response", "expected"), - [ - ({}, {}), - ( +@pytest.mark.parametrize("span_streaming", [True, False]) +def test_service_extension_customizes_client_span( + capture_items, + client_factory, + monkeypatch, + span_streaming, +): + class TestServiceExtension(_ServiceExtension): + def get_span_op(self, ctx): + return "aws.test" + + def get_span_origin(self, ctx): + return "auto.aws.test" + + def get_request_attributes(self, ctx): + return { + "aws.test.request": ctx.params["Key"], + SPANDATA.SENTRY_KIND: "producer", + } + + def get_response_attributes(self, ctx, response): + return { + "aws.test.response": response["ResponseMetadata"]["RequestId"], + SPANDATA.HTTP_STATUS_CODE: 418, + } + + monkeypatch.setitem(_SERVICE_EXTENSIONS, "s3", TestServiceExtension()) + client = client_factory() + api_params = {"Bucket": "bucket", "Key": "foo"} + + with Stubber(client) as stubber: + stubber.add_response( + "head_object", { "ResponseMetadata": { - "RequestId": "request-id", - "HostId": "extended-request-id", "HTTPStatusCode": 200, - "RetryAttempts": 0, - } - }, - { - SPANDATA.AWS_REQUEST_ID: "request-id", - SPANDATA.AWS_EXTENDED_REQUEST_ID: "extended-request-id", - SPANDATA.HTTP_STATUS_CODE: 200, - }, - ), - ( - { - "ResponseMetadata": { "RequestId": "request-id", - "HTTPStatusCode": 200, - "RetryAttempts": 2, + "HostId": "extended-request-id", } }, - { - SPANDATA.AWS_REQUEST_ID: "request-id", - SPANDATA.HTTP_STATUS_CODE: 200, - SPANDATA.HTTP_REQUEST_RESEND_COUNT: 2, - }, - ), - ], -) -def test_get_response_attributes(response, expected): - assert _get_response_attributes(response) == expected - - -@pytest.mark.parametrize( - "header_name", - ["x-amzn-requestid", "x-amzn-request-id", "x-amz-request-id"], -) -def test_get_response_attributes_reads_request_id_header(header_name): - response = { - "ResponseMetadata": { - "HTTPHeaders": {header_name: "request-id"}, - } - } - - assert _get_response_attributes(response) == {SPANDATA.AWS_REQUEST_ID: "request-id"} - - -def test_get_response_attributes_reads_extended_request_id_header(): - response = { - "ResponseMetadata": { - "HTTPHeaders": {"x-amz-id-2": "extended-request-id"}, - } - } - - assert _get_response_attributes(response) == { - SPANDATA.AWS_EXTENDED_REQUEST_ID: "extended-request-id" - } - + api_params, + ) + spans_by_op = _capture_boto3_spans_by_op( + lambda: client.head_object(**api_params), + capture_items, + span_streaming, + expected_origin="auto.aws.test", + ) -@pytest.mark.parametrize( - ("field", "value", "attribute"), - [ - ("RequestId", 123, SPANDATA.AWS_REQUEST_ID), - ("RequestId", "", SPANDATA.AWS_REQUEST_ID), - ("HTTPStatusCode", "200", SPANDATA.HTTP_STATUS_CODE), - ("HTTPStatusCode", True, SPANDATA.HTTP_STATUS_CODE), - ("HTTPStatusCode", 999, SPANDATA.HTTP_STATUS_CODE), - ], -) -def test_get_response_attributes_ignores_malformed_field(field, value, attribute): - metadata = { - "RequestId": "request-id", - "HTTPStatusCode": 200, - "RetryAttempts": 2, - } - metadata[field] = value - - attributes = _get_response_attributes({"ResponseMetadata": metadata}) - expected = { - SPANDATA.AWS_REQUEST_ID: "request-id", - SPANDATA.HTTP_STATUS_CODE: 200, - SPANDATA.HTTP_REQUEST_RESEND_COUNT: 2, - } - expected.pop(attribute) - assert attributes == expected + spans = spans_by_op.get("aws.test", []) + assert len(spans) == 1 + attributes = _span_attributes(spans[0], span_streaming) + assert attributes["aws.test.request"] == "foo" + assert attributes["aws.test.response"] == "request-id" + assert attributes[SPANDATA.SENTRY_KIND] == "producer" + assert attributes[SPANDATA.RPC_METHOD] == "HeadObject" + assert attributes[SPANDATA.HTTP_STATUS_CODE] == 200 + assert attributes[SPANDATA.AWS_EXTENDED_REQUEST_ID] == "extended-request-id" + _assert_span_finished(spans[0], span_streaming) + if span_streaming: + assert attributes[SPANDATA.SENTRY_ORIGIN] == "auto.aws.test" + else: + assert spans[0]["origin"] == "auto.aws.test" @pytest.mark.parametrize( @@ -541,6 +468,13 @@ def test_client_call_has_common_attributes( api_params, capture_items, span_streaming, + response={ + "ResponseMetadata": { + "HTTPStatusCode": 200, + "RequestId": "request-id", + "RetryAttempts": 0, + } + }, ) attributes = _span_attributes(span, span_streaming) @@ -548,9 +482,15 @@ def test_client_call_has_common_attributes( assert attributes[SPANDATA.RPC_SERVICE] == rpc_service assert attributes[SPANDATA.RPC_METHOD] == rpc_method assert attributes[SPANDATA.RPC_SYSTEM_NAME] == AWS_RPC_SYSTEM_NAME + assert attributes[SPANDATA.SENTRY_KIND] == "client" assert attributes[SPANDATA.CLOUD_REGION] == "eu-north-1" assert attributes[SPANDATA.SERVER_ADDRESS] == server_address assert attributes[SPANDATA.SERVER_PORT] == server_port + assert attributes[SPANDATA.HTTP_STATUS_CODE] == 200 + assert attributes[SPANDATA.AWS_REQUEST_ID] == "request-id" + assert SPANDATA.HTTP_REQUEST_RESEND_COUNT not in attributes + assert SPANDATA.ERROR_TYPE not in attributes + _assert_span_finished(span, span_streaming) def test_client_call_attributes_are_available_at_span_creation( @@ -577,10 +517,13 @@ def test_client_call_attributes_are_available_at_span_creation( client = session.client("s3") items = capture_items("span") - with Stubber(client) as stubber: - stubber.add_response("head_object", {}, {"Bucket": "bucket", "Key": "foo"}) - with sentry_sdk.traces.start_span(name="parent"): - client.head_object(Bucket="bucket", Key="foo") + with MockResponse(client, 200, {}, b""): + with sentry_sdk.traces.start_span(name="parent") as parent: + response = client.head_object(Bucket="bucket", Key="foo") + assert response["ResponseMetadata"]["HTTPStatusCode"] == 200 + assert sentry_sdk.traces.get_current_span() is parent + assert SPANDATA.RPC_METHOD not in parent.get_attributes() + assert SPANDATA.HTTP_REQUEST_METHOD not in parent.get_attributes() sentry_sdk.flush() client_spans = [ @@ -591,60 +534,29 @@ def test_client_call_attributes_are_available_at_span_creation( assert client_spans == [] -def test_client_call_omits_missing_region( - sentry_init, - capture_items, - monkeypatch, -): - sentry_init( - traces_sample_rate=1.0, - integrations=[Boto3Integration()], - trace_lifecycle="stream", - server_name="", - ) - client = session.client("s3") - monkeypatch.setattr(type(client.meta), "region_name", property(lambda _: None)) - - span = _capture_stubbed_client_span( - client, - "head_object", - {"Bucket": "bucket", "Key": "foo"}, - capture_items, - span_streaming=True, - ) - - assert SPANDATA.CLOUD_REGION not in span["attributes"] - - @pytest.mark.parametrize("span_streaming", [True, False]) -def test_client_call_has_response_attributes( - capture_items, - client_factory, - span_streaming, +@pytest.mark.parametrize( + "request_id_header", ["x-amzn-requestid", "x-amzn-request-id", "x-amz-request-id"] +) +def test_client_call_has_response_header_attributes( + capture_items, client_factory, span_streaming, request_id_header ): client = client_factory() - span = _capture_stubbed_client_span( - client, - "head_object", - {"Bucket": "bucket", "Key": "foo"}, - capture_items, - span_streaming, - response={ - "ResponseMetadata": { - "HTTPStatusCode": 200, - "RequestId": "request-id", - "HostId": "extended-request-id", - "RetryAttempts": 0, - } - }, - ) - attributes = _span_attributes(span, span_streaming) + headers = {request_id_header: "request-id", "x-amz-id-2": "extended-request-id"} + with MockResponse(client, 200, headers, b""): + spans_by_op = _capture_boto3_spans_by_op( + lambda: client.head_object(Bucket="bucket", Key="foo"), + capture_items, + span_streaming, + ) + spans = spans_by_op[OP.HTTP_CLIENT] + assert len(spans) == 1 + attributes = _span_attributes(spans[0], span_streaming) assert attributes[SPANDATA.HTTP_STATUS_CODE] == 200 assert attributes[SPANDATA.AWS_REQUEST_ID] == "request-id" assert attributes[SPANDATA.AWS_EXTENDED_REQUEST_ID] == "extended-request-id" assert SPANDATA.HTTP_REQUEST_RESEND_COUNT not in attributes - assert SPANDATA.ERROR_TYPE not in attributes @pytest.mark.parametrize("span_streaming", [True, False]) @@ -699,11 +611,24 @@ def attempt_failed_head_object_call(): @pytest.mark.parametrize("span_streaming", [True, False]) +@pytest.mark.parametrize("with_service_extension", [False, True]) def test_client_error_has_response_attributes_and_is_unchanged( capture_items, client_factory, + monkeypatch, span_streaming, + with_service_extension, ): + class TestServiceExtension(_ServiceExtension): + def get_response_attributes(self, ctx, response): + return { + "aws.test.error": response["Error"]["Code"], + SPANDATA.ERROR_TYPE: "must-not-override", + SPANDATA.HTTP_STATUS_CODE: 418, + } + + if with_service_extension: + monkeypatch.setitem(_SERVICE_EXTENSIONS, "s3", TestServiceExtension()) client = client_factory() original_exception = ClientError( { @@ -741,6 +666,8 @@ def invoke_failing_client_method(): assert attributes[SPANDATA.HTTP_STATUS_CODE] == 403 assert attributes[SPANDATA.HTTP_REQUEST_RESEND_COUNT] == 1 assert attributes[SPANDATA.ERROR_TYPE] == "AccessDeniedException" + if with_service_extension: + assert attributes["aws.test.error"] == "AccessDeniedException" assert "Error.Message" not in attributes assert "exception.message" not in attributes assert "error.message" not in attributes @@ -795,23 +722,31 @@ def invoke_failing_client_method(): @pytest.mark.tests_internal_exceptions @pytest.mark.parametrize("span_streaming", [True, False]) -def test_response_attribute_extraction_failure_does_not_change_response( +@pytest.mark.parametrize( + "failing_instrumentation", + [ + "_start_client_span", + "_get_response_attributes", + ], +) +def test_instrumentation_failure_does_not_change_response( capture_items, client_factory, monkeypatch, span_streaming, + failing_instrumentation, ): client = client_factory() api_params = {"Bucket": "bucket", "Key": "foo"} original_response = {"ResponseMetadata": {"HTTPStatusCode": 200}} returned_responses = [] - def fail_attribute_extraction(response): - raise RuntimeError("attribute extraction failed") + def fail_instrumentation(*args, **kwargs): + raise RuntimeError("instrumentation failed") monkeypatch.setattr( - "sentry_sdk.integrations.boto3._client._get_response_attributes", - fail_attribute_extraction, + f"sentry_sdk.integrations.boto3._client.{failing_instrumentation}", + fail_instrumentation, ) def invoke_client_method(): @@ -826,8 +761,11 @@ def invoke_client_method(): client_spans = spans_by_op.get(OP.HTTP_CLIENT, []) assert returned_responses == [original_response] assert returned_responses[0] is original_response - assert len(client_spans) == 1 - _assert_span_finished(client_spans[0], span_streaming) + if failing_instrumentation == "_get_response_attributes": + assert len(client_spans) == 1 + _assert_span_finished(client_spans[0], span_streaming) + else: + assert client_spans == [] @pytest.mark.tests_internal_exceptions