opentelemetry-python-contrib
opentelemetry-python-contrib copied to clipboard
User provided callbacks should have exceptions checked
When updating httpx instrumentation, I found there was a (likely unintended) breaking change.
https://github.com/open-telemetry/opentelemetry-python-contrib/pull/2020#discussion_r1505190185
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,
)
Related behavior in the Django instrumentor: https://github.com/open-telemetry/opentelemetry-python-contrib/pull/2153
@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 https://github.com/open-telemetry/opentelemetry-python-contrib/pull/2359