feat(httpx2): add httpx2 instrumentation - #19792
Conversation
Circular import analysis
|
Dependency direction analysis
|
Codeowners resolved asResolved from the full PR diff against |
🎉 All green!🧪 All tests passed 🔄 Datadog auto-retried 3 jobs - 3 passed on retry 🔗 Commit SHA: 5e06c02 | Docs | View more details | Give us feedback! |
BenchmarksBenchmark execution time: 2026-08-20 15:53:22 Comparing candidate commit 5e06c02 in PR branch Found 0 performance improvements and 9 performance regressions! Performance is the same for 611 metrics, 10 unstable metrics.
|
![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: 5e06c0235b
ℹ️ About Codex in GitHub
Codex has been enabled to automatically 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 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| "gevent": True, | ||
| "graphql": True, | ||
| "grpc": True, | ||
| "httpx2": True, |
There was a problem hiding this comment.
When an application calls httpx2.alias_httpx() and sets DD_TRACE_HTTPX2_ENABLED=false (or calls patch_all(httpx2=False)), this entry is disabled but the existing httpx hook remains active. Because httpx then refers to the same aliased module, the HTTPX patch still wraps its clients and emits spans using config.httpx, so users cannot disable HTTPX2 instrumentation without also disabling the separate HTTPX integration. Coordinate the hooks or detect the aliased module so the HTTPX2 enablement setting remains authoritative.
AGENTS.md reference: AGENTS.md:L59-L62
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Man this is self sabotaging if someone is doing that
| with core.context_with_event( | ||
| event=HttpClientSendEvent( | ||
| request_url=httpx_url_to_str(request.url), | ||
| request_method=request.method, | ||
| request_headers=request.headers, | ||
| request_body=lambda: request.content, | ||
| ), | ||
| context_name_override=HttpClientEvents.HTTPX_SEND_REQUEST.value, | ||
| ) as ctx: | ||
| response = None | ||
| try: | ||
| response = wrapped(*args, **kwargs) | ||
| return response | ||
| finally: | ||
| if response is not None: | ||
| ctx.event.set_response(response) | ||
| return None |
There was a problem hiding this comment.
only comment on the pr is that these are literally line by line the same as the current patch methods in httpx, wondering if we should de dedupe.
There was a problem hiding this comment.
Right now, it would make sense but if httpx2 start to deviate from httpx it could complexify the work so I think I'd prefer to duplicate but this is a personal preference and I don't have super strong opinions
| with core.context_with_event( | ||
| HttpClientRequestEvent( | ||
| http_operation="http.request", | ||
| service=httpx_get_service_name(request, config.httpx2), | ||
| component=config.httpx2.integration_name, | ||
| request_method=request.method, | ||
| request_headers=request.headers, | ||
| integration_config=config.httpx2, | ||
| request_url=httpx_url_to_str(request.url), | ||
| query=ensure_text(request.url.query), | ||
| target_host=request.url.host, | ||
| ), | ||
| context_name_override=HttpClientEvents.HTTPX_REQUEST.value, | ||
| ) as ctx: | ||
| response = None | ||
| try: | ||
| response = await wrapped(*args, **kwargs) | ||
| return response | ||
| finally: |
There was a problem hiding this comment.
same for all the patch functions, tho this would need config passed in
wconti27
left a comment
wconti27
left a comment
There was a problem hiding this comment.
1 nit around patches being duped code from httpx to httpx2, may want to consider merging those code paths
| query=ensure_text(request.url.query), | ||
| target_host=request.url.host, | ||
| ), | ||
| context_name_override=HttpClientEvents.HTTPX_REQUEST.value, |
There was a problem hiding this comment.
Would it be possible to use a dedicated HTTPX2_REQUEST event name initially? (same for HTTPX2_SEND_REQUEST)
This would send events to the AppSec subscriber even though we don’t currently have test coverage confirming that httpx2 works with AppSec.
Since this is a fork of httpx, I expect it to work and to be semantically correct, but still I’d prefer to keep the two separate until that has been properly tested.
Would be happy to unify both again once I have added the necessary test coverage.
3 participants
Fulfill #18780
Description
Add
httpx2integration. httpx2 is a fork of httpx (which is not unmaintained) so the implementation is almost if not completely the same ashttpxintegration.It seems httpx2 is compatible with websocket so it might be worth supporting that in a follow up