open-telemetry / open-telemetry/opentelemetry-python-contrib

User provided callbacks should have exceptions checked

Open
#2,305 2 comments 3 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

bug
Dominant language
Python
Stars
1.1k
Forks
1.1k
Avg merge
4d 15h
Merged PRs (30d)
16

Description

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,
    )

Contributor guide

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Start at HTTPXClientInstrumentor.instrument and the request_hook and async_request_hook call sites, then compare their behavior with the linked OpenTelemetry error-handling specification. Done means exceptions from user-provided callbacks no longer propagate into the application, with the intended logging behavior covered by tests.

Written by the indexing model from the issue text.

Assessment

Tech stack
python
Domain
observability-sre
Issue type
Bug
Difficulty
4/5
Estimated time
3-5 days
Activity status
Stale
Clarity
Mostly clear
Newbie friendliness
38/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.