diff --git a/instrumentation/opentelemetry-instrumentation-botocore/src/opentelemetry/instrumentation/botocore/__init__.py b/instrumentation/opentelemetry-instrumentation-botocore/src/opentelemetry/instrumentation/botocore/__init__.py index 0ff08628b7..fa1347f2ff 100644 --- a/instrumentation/opentelemetry-instrumentation-botocore/src/opentelemetry/instrumentation/botocore/__init__.py +++ b/instrumentation/opentelemetry-instrumentation-botocore/src/opentelemetry/instrumentation/botocore/__init__.py @@ -157,6 +157,7 @@ def response_hook(span, service_name, operation_name, result): RPC_SYSTEM, ) from opentelemetry.trace.span import Span +from opentelemetry.trace.status import StatusCode logger = logging.getLogger(__name__) @@ -279,6 +280,7 @@ def _patched_api_call(self, original_func, instance, args, kwargs): # tracing streaming services require to close the span manually # at a later time after the stream has been consumed end_on_exit=end_span_on_exit, + set_status_on_exception=False, ) as span: _safe_invoke(extension.before_service_call, span, instrumentor_ctx) self._call_request_hook(span, call_context) @@ -291,9 +293,25 @@ def _patched_api_call(self, original_func, instance, args, kwargs): except ClientError as error: result = getattr(error, "response", None) _apply_response_attributes(span, result) - _safe_invoke( - extension.on_error, span, error, instrumentor_ctx - ) + if _is_http_error(result): + span.set_status(StatusCode.ERROR, str(error)) + _safe_invoke( + extension.on_error, + span, + error, + instrumentor_ctx, + ) + else: + span.set_status(StatusCode.OK) + _safe_invoke( + extension.on_success, + span, + result, + instrumentor_ctx, + ) + raise + except Exception as error: + span.set_status(StatusCode.ERROR, str(error)) raise _apply_response_attributes(span, result) _safe_invoke( @@ -439,6 +457,7 @@ async def _patched_api_call(self, original_func, instance, args, kwargs): # tracing streaming services require to close the span manually # at a later time after the stream has been consumed end_on_exit=end_span_on_exit, + set_status_on_exception=False, ) as span: _safe_invoke(extension.before_service_call, span, instrumentor_ctx) self._call_request_hook(span, call_context) @@ -451,9 +470,25 @@ async def _patched_api_call(self, original_func, instance, args, kwargs): except ClientError as error: result = getattr(error, "response", None) _apply_response_attributes(span, result) - _safe_invoke( - extension.on_error, span, error, instrumentor_ctx - ) + if _is_http_error(result): + span.set_status(StatusCode.ERROR, str(error)) + _safe_invoke( + extension.on_error, + span, + error, + instrumentor_ctx, + ) + else: + span.set_status(StatusCode.OK) + _safe_invoke( + extension.on_success, + span, + result, + instrumentor_ctx, + ) + raise + except Exception as error: + span.set_status(StatusCode.ERROR, str(error)) raise _apply_response_attributes(span, result) _safe_invoke( @@ -485,6 +520,16 @@ def _call_response_hook( ) +def _is_http_error(result: Optional[Dict[str, Any]]) -> bool: + """Return True if the ClientError response represents a genuine HTTP error (>= 400).""" + if result is None: + return True + status_code = result.get("ResponseMetadata", {}).get("HTTPStatusCode") + if status_code is None: + return True + return status_code >= 400 + + def _apply_response_attributes(span: Span, result): if result is None or not span.is_recording(): return diff --git a/instrumentation/opentelemetry-instrumentation-botocore/tests/test_botocore_instrumentation.py b/instrumentation/opentelemetry-instrumentation-botocore/tests/test_botocore_instrumentation.py index de5909e85c..ba2c665167 100644 --- a/instrumentation/opentelemetry-instrumentation-botocore/tests/test_botocore_instrumentation.py +++ b/instrumentation/opentelemetry-instrumentation-botocore/tests/test_botocore_instrumentation.py @@ -7,7 +7,7 @@ from urllib.parse import urlparse import botocore.session -from botocore.exceptions import ParamValidationError +from botocore.exceptions import ClientError, ParamValidationError from moto import mock_aws # pylint: disable=import-error from opentelemetry import trace as trace_api @@ -167,6 +167,44 @@ def test_exception(self): self.assertIn(EXCEPTION_TYPE, event.attributes) self.assertIn(EXCEPTION_MESSAGE, event.attributes) + @mock_aws + def test_client_error_with_http_error_status(self): + """ClientError with HTTP 4xx sets span status to ERROR.""" + s3 = self._make_client("s3") + + with self.assertRaises(ClientError): + s3.get_object(Bucket="non-existent-bucket", Key="non-existent-key") + + spans = self.memory_exporter.get_finished_spans() + self.assertEqual(1, len(spans)) + span = spans[0] + self.assertIs(span.status.status_code, trace_api.StatusCode.ERROR) + + @mock_aws + def test_client_error_304_not_modified(self): + """S3 IfNoneMatch returning 304 sets span status to OK.""" + s3 = self._make_client("s3") + s3.create_bucket( + Bucket="test-bucket", + CreateBucketConfiguration={"LocationConstraint": self.region}, + ) + etag = s3.put_object(Bucket="test-bucket", Key="key", Body=b"data")[ + "ETag" + ] + + with self.assertRaises(ClientError): + s3.get_object( + Bucket="test-bucket", Key="key", IfNoneMatch=etag + ) + + spans = self.memory_exporter.get_finished_spans() + # Filter to the GetObject span + get_spans = [s for s in spans if s.name == "S3.GetObject"] + self.assertEqual(1, len(get_spans)) + self.assertIs( + get_spans[0].status.status_code, trace_api.StatusCode.OK + ) + @mock_aws def test_s3_client(self): s3 = self._make_client("s3")