feat(traces): span batch transport - #952
turnipdabeets wants to merge 2 commits into
Conversation
Prompt To Fix All With AI### Issue 1
posthog/tracing/_transport.py:101-111
**Redirects can discard traces**
If a custom or self-hosted ingestion endpoint returns a 301, 302, or 303 redirect, `requests` follows it by default and may rewrite the POST as a GET. The final 2xx response is then reported as success even though the OTLP body was not ingested. Disable implicit redirects, or explicitly follow only bounded, same-origin 307/308 redirects while preserving the POST.
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.Reviews (1): Last reviewed commit: "feat(traces): span batch transport" | Re-trigger Greptile |
posthog-python Compliance ReportDate: 2026-09-15 22:09:29 UTC ✅ All Tests Passed!111/111 tests passed Capture_V1 Tests✅ 94/94 tests passed View Details
Feature_Flags Tests✅ 17/17 tests passed View Details
|
1f1d603 to
13c8282
Compare
13c8282 to
d33a226
Compare
d33a226 to
8f5b3c1
Compare
Adds the HTTP transport: one gzipped OTLP JSON POST per batch to
{host}/i/v1/traces with bearer auth, classified as ok (2xx), too large (413, or
a body over the 10 MiB hosted ingestion limit, refused without a request),
retriable (408, 429, 5xx, transport errors, carrying any Retry-After as
delta-seconds or HTTP-date) or fatal (other 4xx). Not reachable from the client.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01TkZAsCciW4PV8ZdcCHmAbA
8f5b3c1 to
fbbbe38
Compare
|
Reviews (2): Last reviewed commit: "feat(traces): span batch transport" | Re-trigger Greptile |
dustinbyrne
left a comment
There was a problem hiding this comment.
One transport issue to address before the public tracing wiring:
At posthog/tracing/_transport.py:101–110, this POST downloads the entire response body even though classification uses only status and headers. Requests' timeout bounds read inactivity, not total response duration. A proxy returning 503 headers followed by an unfinished body with periodic chunks can keep this call open indefinitely instead of returning retry-later. The intended exporter holds its single-flight slot across the call.
Please classify a streamed response and reliably close it without consuming the unused body. That fixes this body-related hang; it does not by itself prove a whole-request deadline, particularly across redirects.
Regression sketch (not executed): use a local server returning 503 with a chunked body that sends every 10 ms without finishing; use a 0.5-second client timeout and assert the transport returns retry-later within two seconds. A completed 200 response is the negative control. Release the server loop during cleanup.
This is a private foundation issue, not a currently shipped Client tracing regression.
AI-assisted review with independent validation of source, pinned Requests/urllib3 implementations and existing CI; no new tests were run.
A requests timeout bounds read inactivity, not the whole response, so a proxy answering 503 with a body that keeps dripping held the exporter's single flight open indefinitely. Stream the response, classify it from status and headers, and close it unread.
|
Good catch, fixed in 12bc88f. We now read only the status and headers, then close the response without reading the body. Added a test with a local server that returns 503 and keeps sending chunks forever. It hung before and now returns retry-later in under two seconds. |
dustinbyrne
left a comment
There was a problem hiding this comment.
Thanks, this addresses my transport finding. I checked the streamed response, close in finally, and the new dripping-body regression test in 12bc88f. Current CI is green. Approving.
AI-assisted source and existing-CI review; I did not run new tests locally.
💡 Motivation and Context
Adds the HTTP transport: one gzipped OTLP JSON POST per batch to
{host}/i/v1/traceswith bearer auth. Responses are classified as:Retry-Afteras delta-seconds or HTTP-dateNot reachable from the client yet.
Stack (PR 4 of 9, based on
traces/03-span-handles):traces/01-ids-traceparenttraces/02-otlp-encodingtraces/03-span-handlestraces/04-transport← this PRtraces/05-pipelinetraces/06-exporttraces/07-span-limitstraces/08-before-span-sendtraces/09-client-wiring💚 How did you test it?
Unit tests in
posthog/test/tracing/test_transport.pycover the request shape, every response class andRetry-Afterparsing.📝 Checklist
If releasing new changes
sampo addto generate a changeset file🤖 Agent context
Autonomy: Human-driven (agent-assisted)
Implemented with Claude Code (Claude Opus 5) against the traces spec, one commit per slice so each PR reviews on its own. Rebased onto main and opened as a stacked draft in a later Claude Code session (Claude Fable 5.1).
🤖 Generated with Claude Code
https://claude.ai/code/session_012o7CtHLfcypjmXL7g9ZGRC