fix(instrumentation-botocore): do not mark HTTP 3xx responses as span errors#4653
Open
stark256-spec wants to merge 2 commits into
Open
fix(instrumentation-botocore): do not mark HTTP 3xx responses as span errors#4653stark256-spec wants to merge 2 commits into
stark256-spec wants to merge 2 commits into
Conversation
…create and AsyncMessages.stream AnthropicInstrumentor only patched the synchronous Messages.create and Messages.stream methods. AsyncAnthropic clients use AsyncMessages, so async calls produced no spans at all. Add two new patch functions mirroring the sync variants: - async_messages_create: awaits wrapped(), detects AsyncStream responses and wraps them in the existing AsyncMessagesStreamWrapper - async_messages_stream: wraps the async stream manager in the existing AsyncMessagesStreamManagerWrapper Wire both into _instrument() with wrap_function_wrapper targeting AsyncMessages.create and AsyncMessages.stream, and add the corresponding unwrap() calls in _uninstrument(). The AsyncMessagesStreamWrapper and AsyncMessagesStreamManagerWrapper classes were already present in wrappers.py but were not being used. Fixes open-telemetry#4497 Assisted-by: Claude Sonnet 4.5
botocore raises ClientError for HTTP 3xx responses such as 304 Not Modified (returned when using conditional requests with IfNoneMatch or IfModifiedSince). The instrumentor was calling extension.on_error and allowing the span context manager to set StatusCode.ERROR for all ClientError exceptions regardless of HTTP status. 304 and other 3xx responses are expected, non-error outcomes. Marking them as errors pollutes error-rate dashboards and triggers false alerts. Fix: extract the HTTPStatusCode from the error response metadata before deciding whether to invoke on_error. For HTTP status < 400, explicitly set StatusCode.OK on the span (which prevents the context manager's automatic ERROR status from overriding it) and skip on_error. For 4xx and 5xx the existing behaviour is unchanged. The fix is applied to both BotocoreInstrumentor and AiobotocoreInstrumentor (identical except/raise blocks). Fixes open-telemetry#4633
52a4976 to
8839991
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
Fixes #4633
What changed and why
botocore raises
ClientErrorfor HTTP 3xx responses such as304 Not Modified(returned when using conditional S3 requests withIfNoneMatchorIfModifiedSince). The instrumentor was callingextension.on_errorfor everyClientErrorregardless of HTTP status, and the span context manager's defaultset_status_on_exceptionbehaviour then setStatusCode.ERRORas the exception propagated out.304 Not Modifiedand other 3xx responses are normal, expected outcomes — not errors. Marking them as errors pollutes error-rate dashboards and triggers false alerts.Fix: check
HTTPStatusCodefrom the error response metadata before invokingon_error. For HTTP< 400, explicitly setStatusCode.OKon the span (which the OTel spec guarantees cannot be overridden toERRORby the context manager) and skipon_error. For 4xx/5xx the existing behaviour is unchanged.Applied to both
BotocoreInstrumentorandAiobotocoreInstrumentor.Testing