open-telemetry / opentelemetry-python-contrib

OpenTelemetry instrumentation for Python modules
https://opentelemetry.io
Apache License 2.0
737 stars 607 forks source link

requests package treats >= 300 status code as an error #2165

Open jbaxleyiii opened 9 months ago

jbaxleyiii commented 9 months ago

Describe your environment python 3.9 opentelemetry-api==1.22.0 opentelemetry-instrumentation-requests==0.43b0

Steps to reproduce Make an http call with requests and get back any http status code >= 300

What is the expected behavior? This could very much be a misunderstanding on my part! When making an http call with requests, I would expect only >= 500 status codes to be treated as exceptions on traces but right now all codes above 299 are treated as exceptions. I didn't expect redirects or 400s to be logged as errors on my traces.

What is the actual behavior? In the requests library, when it creates a span it sets the type to be client (https://github.com/open-telemetry/opentelemetry-python-contrib/blob/8fd2105ceae604cf39a67f1c4dd154753b43fcd1/instrumentation/opentelemetry-instrumentation-requests/src/opentelemetry/instrumentation/requests/__init__.py#L208-L210). When it converts this to an otel status code, it uses:

def http_status_to_status_code(
    status: int,
    allow_redirect: bool = True,
    server_span: bool = False,
) -> StatusCode:
    """Converts an HTTP status code to an OpenTelemetry canonical status code

    Args:
        status (int): HTTP status code
    """
    # See: https://github.com/open-telemetry/opentelemetry-specification/blob/main/specification/trace/semantic_conventions/http.md#status
    if not isinstance(status, int):
        return StatusCode.UNSET

    if status < 100:
        return StatusCode.ERROR
    if status <= 299:
        return StatusCode.UNSET
    if status <= 399 and allow_redirect:
        return StatusCode.UNSET
    if status <= 499 and server_span:
        return StatusCode.UNSET
    return StatusCode.ERROR

and doesn't pass in a server_span so all non 2** status codes are treated as errors.

federicobond commented 9 months ago

If I understand correctly from the code you posted, an HTTP status 3xx would only set the status to ERROR if allow_redirect=False, which is not by default. 4xx status means client errors so they are treated as errors if inside a client span which is expected.