Skip to content
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.

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

User provided callbacks should have exceptions checked #2305

Open
anuraaga opened this issue Feb 28, 2024 · 2 comments
Open

User provided callbacks should have exceptions checked #2305

anuraaga opened this issue Feb 28, 2024 · 2 comments
Labels
bug Something isn't working

Comments

@anuraaga
Copy link

anuraaga commented Feb 28, 2024

When updating httpx instrumentation, I found there was a (likely unintended) breaking change.

#2020 (comment)

I found it because my app broke when it crashed on trying to read url.host in a request hook. AFAIK, this is incompliant with the spec which expects instrumentation to never cause app crashes, even in the face of broken user callbacks. Probably any user callback needs to be wrapped in a exception handler that optionally logs the error without failing completely.

https://github.com/open-telemetry/opentelemetry-specification/blob/6fd4f0809bcff0ea5c4abe161803b4ad8628375e/specification/error-handling.md?plain=1#L24

The code that crashed, looks like

async def _httpx_span_name_hook(span: Span, request: RequestInfo):
    span.update_name(f"{request.method.decode()} {request.url.host}")

def _httpx_span_name_hook_sync(span: Span, request: RequestInfo):
  span.update_name(f"{request.method.decode()} {request.url.host}")

HTTPXClientInstrumentor().instrument(
        async_request_hook=_httpx_span_name_hook,
        request_hook=_httpx_span_name_hook_sync,
    )
@anuraaga anuraaga added the bug Something isn't working label Feb 28, 2024
@ebracho
Copy link
Contributor

ebracho commented Feb 29, 2024

Related behavior in the Django instrumentor: #2153

@samypr100
Copy link
Contributor

@anuraaga Even though this issue is for a general ask, the aformentioned bug that led you to open this issue should be fixed now in #2359

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

3 participants