feat(traces): span handles - #951
Conversation
posthog-python Compliance ReportDate: 2026-09-17 03:43:06 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
|
Prompt To Fix All With AI### Issue 1
posthog/tracing/_span.py:333-339
**Shared spans end early**
The implementation supports entering one span from several threads or tasks, but each `__exit__` ends that shared span. If one block exits while another is still active, later events and exceptions from the remaining block are ignored, producing an incomplete trace. Track active scope ownership and end only after the final scope exits, or explicitly reject concurrent entry.
### Issue 2
posthog/tracing/_span.py:302-305
**Concurrent end exports duplicates**
`end()` checks and sets `_ended` without synchronization. Concurrent callers can both pass the check and invoke `_on_end`, violating the documented idempotency contract and exporting duplicate span records. Protect the check-and-set transition with a lock.
---
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 handles" | Re-trigger Greptile |
8848500 to
f506a3e
Compare
f506a3e to
af28b90
Compare
ece21ff to
cf74b0b
Compare
|
Reviews (2): Last reviewed commit: "feat(traces): span handles" | Re-trigger Greptile |
dustinbyrne
left a comment
There was a problem hiding this comment.
Reviewed cf74b0b17bcf0e496fd4f35b9a63837f4e6a7cf2 via source inspection and existing CI. No issues found in this staged span-handle slice. Approval covers this slice; #950 precedes it in the stack.
jzhu13
left a comment
There was a problem hiding this comment.
Reviewed against traces/02-otlp-encoding. Tests pass at the head, ruff and mypy are clean, and the ContextVar token handling held up under nested, threaded, async, and copied-context probes. Two items I would fix before merge.
Blocking
posthog/tracing/_span.py:335__exit__records everyBaseExceptionas an error span with an exception event:GeneratorExit,asyncio.CancelledError,KeyboardInterrupt,SystemExit. Reproduced: a generator holdingwith span:that is closed early yieldsSpanStatus('error')andexception.type='GeneratorExit'. A streaming endpoint whose consumer breaks out of the loop, or any request cancelled on client disconnect, ships as a failure. OTel'suse_spancatchesExceptiononly for this reason. Suggestisinstance(exc, Exception)for the record-and-error path while still callingend()unconditionally, tighten thespan.pydocstrings from "an exception" to "anException", and add a parametrized case forGeneratorExitandCancelledError.posthog/tracing/_span.py:333end()runs before_deactivate(), soon_end(and, from #956, thebefore_span_sendhook) sees the ending span as still active. Reproduced at the stack tip: a hook that callsstart_spanduring scoped exit getsparent_span_id == outer.span_id, a child that starts after its parent ended. OTel detaches first, then ends. Swap the order.
Non-blocking
posthog/tracing/_span.py:302end()idempotence is a non-atomic check-then-set on_ended. Two threads ending one handle can callon_endtwice, contradicting the public "Idempotent" docstring. Wrapping the flag flip in the existing_tokens_lockmakes the guarantee the handle's own rather than #953's.posthog/tracing/_span.py:76_deactivateis a silent no-op when no token matches the current context (exit in acopy_context()child, or a thread that entered and died). The span stays active and a stale token stays in the list. A debug log would make "why is my span still active" diagnosable.posthog/tracing/span.py:24saysNoneremoves the key, but at this PR the record storesNoneand only the encoder drops it. #955 makes the sentence true. Either pop here or move the sentence.- Nits:
Spanis never rendered bybin/docs_scripts/generate_json_schemas.py, which only visitsClientand module functions, so #957'sstart_spandocuments a return type with no page; no__slots__on handles that carry ~19 attributes with a default cap of 10000 live spans; the PR body cites a spec but links no issue, which CONTRIBUTING asks for on public API changes.
Reviewed with Claude Code (Claude Fable 5.1). Behaviors above were reproduced by probe against this branch head or the stack tip where noted.
cf74b0b to
ceddd18
Compare
|
Thanks. Fixed in ceddd18:
|
ceddd18 to
c37bdb0
Compare
|
Follow-up: nothing further changed here since ceddd18; the stack below it was rebased. Ready for another look. |
Adds the Span interface (posthog.tracing.span, the only public module in the package) and its handles. The recording span keeps its timing on a monotonic clock, with children of a local parent on the root's clock basis so they stay inside it; end() is idempotent and hands the pipeline one record. The no-op handle is returned when tracing cannot run, and the pass-through handle echoes an inbound traceparent and tracestate so a service with tracing off still forwards the trace, including from spans nested inside it. Entering a handle makes it the active span for the block and records an exception raised out of it. Not reachable from the client. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01TkZAsCciW4PV8ZdcCHmAbA
… exit A generator closed early or a cancelled task raised GeneratorExit or CancelledError through __exit__ and shipped as an error span. The scoped form now records an Exception only, still ending on every exit. The span is detached before end() so nothing started from the on_end path nests under a span that is over. end() flips its flag under the handle's lock, and a deactivate that finds no token for the context says so at debug.
c37bdb0 to
2aa0efb
Compare
💡 Motivation and Context
Adds the
Spaninterface (posthog.tracing.span, the only public module in the package) and its three handles:end()is idempotent and hands the pipeline one record.traceparent/tracestateso a service with tracing off still forwards the trace, including from spans nested inside it.Entering a handle makes it the active span for the block and records an exception raised out of it. Not reachable from the client yet.
Stack (PR 3 of 9, based on
traces/02-otlp-encoding):traces/01-ids-traceparenttraces/02-otlp-encodingtraces/03-span-handles← this PRtraces/04-transporttraces/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_span.pycover timing, idempotentend(), active-span scoping, exception recording and pass-through propagation.📝 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