Add native OpenTelemetry client telemetry#1052
Conversation
|
Docs preview: |
aa1ba0f to
653b450
Compare
Merging this PR will not alter performance
Comparing 
Footnotes
|
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: aa1ba0fc4b
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
1 issue found across 3 files (changes from recent commits).
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
d493ddb to
4f5477d
Compare
4f5477d to
28cde97
Compare
| otel = get_opentelemetry() | ||
| if otel is None or not otel.is_enabled(request): | ||
| response = transport.handle_request(request) | ||
| else: | ||
| trace = otel.start_request(request) | ||
| try: | ||
| response = transport.handle_request(request) | ||
| trace.set_response(response) | ||
| except BaseException as exc: | ||
| trace.set_exception(exc) | ||
| trace.detach_current(type(exc), exc, exc.__traceback__) | ||
| trace.close() | ||
| raise | ||
| trace.detach_current() | ||
| if response.is_closed: | ||
| trace.close() | ||
| else: | ||
| assert isinstance(response.stream, SyncByteStream) | ||
| response.stream = trace.wrap_sync_stream(response.stream) |
There was a problem hiding this comment.
This seems extremely complex. We are not merging this PR with this. I'll see what to do here.
There was a problem hiding this comment.
1 issue found across 3 files (changes from recent commits).
Tip: Review your code locally with the cubic CLI to iterate faster.
Re-trigger cubic
There was a problem hiding this comment.
1 issue found across 2 files (changes from recent commits).
Tip: Review your code locally with the cubic CLI to iterate faster.
Re-trigger cubic
sk-
left a comment
sk-
left a comment
There was a problem hiding this comment.
Awesome work @Kludex.
I have some comments:
- could you add a section to the PR description showing the overhead of this solution
- would it be possible to only register metrics or traces. In my company we only use metrics, and would like to avoid the extra overhead of the traces.
- could you consider a way to specify url.template. This is a very useful opt-in attribute that allows to detect issues in specific API endpoints.
2 participants
Summary
Adds native OpenTelemetry client telemetry for
httpx2without adding a runtime dependency unless users install theopentelemetryextra.opentelemetry-apiand emits current stable HTTP client spans andhttp.client.request.durationmetrics.httpx2[opentelemetry]withopentelemetry-apionly.Validation
scripts/checkuv run pytest tests/httpx2 -quv run pytest tests/httpx2/test_opentelemetry.py -q