getsentry / sentry-python

The official Python SDK for Sentry.io
https://sentry.io/for/python/
MIT License
1.9k stars 503 forks source link

Sentry does not fully instrument `requests` library requests #2277

Open filips123 opened 1 year ago

filips123 commented 1 year ago

How do you use Sentry?

Sentry Saas (sentry.io)

Version

1.28.1

Steps to Reproduce

I made an example program to show this error:

from hashlib import sha256

import requests
import sentry_sdk
from sentry_sdk.integrations.flask import FlaskIntegration
from sentry_sdk.integrations.pure_eval import PureEvalIntegration
from sentry_sdk.integrations.sqlalchemy import SqlalchemyIntegration

def download() -> tuple[str, str]:
    """Download the timetable JS file."""

    response = requests.get("https://projekti.gimvic.org/urnik/data.js")
    response.raise_for_status()
    content = response.content

    return content.decode("utf8"), sha256(content).hexdigest()

def main():
    sentry_sdk.init(
        dsn="DSN",
        max_breadcrumbs=100,
        traces_sample_rate=1,
        profiles_sample_rate=1,
        integrations=[
            FlaskIntegration(transaction_style="url"),
            SqlalchemyIntegration(),
            PureEvalIntegration(),
        ],
        environment="production",
        release="0.0.0",
    )

    with sentry_sdk.start_transaction(name="test") as transaction:
        # Download the document
        content, sha = download()

        # Do some other processing
        with transaction.start_child(op="work") as child:
            child.set_tag("hash", sha)
            print(sha)

main()

The code in production is different and more complicated as it's integrated into a larger project. However, the same problem still happens.

Interestingly, it does not happen with all URLs, but it happens consistently with this URL (and a few others).

Expected Result

Sentry should auto-instrument all requests code and assign http.client span.

Actual Result

Sentry instruments only a part of requests.get as http.client. The rest of the HTTP request handling does not have a span assigned, which can make Sentry show "Missing span instrumentation", even though the profiler shows that the code is related to the HTTP request and comes from the requests library.

Screenshots of traces and running my example program:

traces profile

Links to a few traces/profiles running my example program:

Link to a few traces/profiles showing the problem in production:

sentrivana commented 1 year ago

Hey @filips123, thanks a ton for the super detailed bug report, this will make it much easier for us to debug! We're a bit swamped at the moment but will take a look when time permits.

antonpirker commented 1 year ago

Looks like reading a big chunked response using response.content is not instrumented and this is the thing that causes the "missing instrumentation" (maybe, I just looked at it very briefly)

szokeasaurusrex commented 5 months ago

We should not add a requests integration; rather, we should improve our support for instrumenting streaming responses