Skip to content

Don't error on 3xx responses #4851

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.

Sign up for GitHub

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

Draft
alex-pinkney-trp wants to merge 2 commits into
base: main
Choose a base branch
from
+90 −7
Draft

Don't error on 3xx responses #4851

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
7a8e1ed
botocore: Don't emit errors on 3xx responses
alex-pinkney-trp May 26, 2026
57f22f7
Merge remote-tracking branch 'trp-fork/main' into dont-error-on-3xx-r…
alex-pinkney-trp Jul 22, 2026
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
57 changes: 51 additions & 6 deletions ...telemetry-instrumentation-botocore/src/opentelemetry/instrumentation/botocore/__init__.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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__)

Expand Down Expand Up @@ -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)
Expand All @@ -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(
Expand Down Expand Up @@ -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)
Expand All @@ -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(
Expand Down Expand Up @@ -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
Expand Down
40 changes: 39 additions & 1 deletion ...rumentation/opentelemetry-instrumentation-botocore/tests/test_botocore_instrumentation.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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")
Expand Down