Skip to content

Commit be93a2c

Browse files
fix(traces): keep shutdown and fork isolation airtight for the client
Shutdown reads the pipeline under the same lock initialization publishes it with, so an initialization in flight is either closed by shutdown or sees the request and stays off. A forked child gets a fresh active-span variable: an inherited handle exiting in the child resets the old one, which would have restored the parent process's outer span.
1 parent 41d6980 commit be93a2c

3 files changed

Lines changed: 47 additions & 6 deletions

File tree

posthog/client.py

Lines changed: 10 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -2276,9 +2276,11 @@ def _reinit_after_fork(self):
22762276
if self._metrics is not None:
22772277
self._metrics._reinit_after_fork()
22782278
self._traces_lock = threading.Lock()
2279+
# A fresh variable: an inherited handle resets the old one on exit,
2280+
# which would restore the parent process's outer span.
2281+
self._active_span_var = ContextVar("posthog_active_span", default=None)
22792282
if self._traces is not None:
2280-
self._traces.reinit_after_fork()
2281-
self._active_span_var.set(None)
2283+
self._traces.reinit_after_fork(self._active_span_var)
22822284

22832285
# If using Redis cache, we must reinitialize to get a fresh connection (fork-safe).
22842286
# If using Memory cache, we keep it as-is to benefit from the inherited warm cache.
@@ -2513,7 +2515,9 @@ def _traces_pipeline(self) -> Optional[PostHogTraces]:
25132515
return self._traces
25142516
if self._traces is None:
25152517
with self._traces_lock:
2516-
if self._traces is None:
2518+
# Re-checked under the lock, which shutdown takes before it
2519+
# reads the pipeline, so neither can miss the other.
2520+
if self._traces is None and not self._shutdown_requested:
25172521
try:
25182522
config = resolve_traces_config(
25192523
self._traces_config, host_resource_attributes()
@@ -2878,15 +2882,16 @@ def _shutdown_once(self, errors: list[Exception]) -> None:
28782882
self._run_lifecycle_cleanup(
28792883
"Failed to reset metrics on shutdown", self._metrics.reset, errors
28802884
)
2881-
if self._traces is not None:
2885+
with self._traces_lock:
28822886
traces = self._traces
2887+
if traces is not None:
28832888
self._run_lifecycle_cleanup(
28842889
"Failed to flush spans on shutdown",
28852890
lambda: traces.flush(_TRACES_SHUTDOWN_FLUSH_SECONDS),
28862891
errors,
28872892
)
28882893
self._run_lifecycle_cleanup(
2889-
"Failed to close traces on shutdown", self._traces.close, errors
2894+
"Failed to close traces on shutdown", traces.close, errors
28902895
)
28912896
self._join_once(errors, flush_queues=False, lanes_prepared=True)
28922897
self._run_lifecycle_cleanup(

posthog/test/tracing/test_client_traces.py

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -460,6 +460,24 @@ def test_tracing_is_inert_after_shutdown(self, no_timers):
460460
assert client.start_span("late") is NOOP_SPAN
461461
assert client._traces._exporter._flush_timer is None
462462

463+
def test_shutdown_closes_a_pipeline_still_initializing(self, no_timers):
464+
client = make_client(traces={})
465+
shutdown = threading.Thread(target=client.shutdown)
466+
resolve = posthog.client.resolve_traces_config
467+
468+
def resolve_while_shutting_down(*args):
469+
shutdown.start()
470+
time.sleep(0.05)
471+
return resolve(*args)
472+
473+
with mock.patch(
474+
"posthog.client.resolve_traces_config", resolve_while_shutting_down
475+
):
476+
client.start_span("racing").end()
477+
shutdown.join()
478+
assert client._traces._closed
479+
assert client._traces._exporter._flush_timer is None
480+
463481
def test_tracing_never_starts_after_shutdown(self, no_timers):
464482
client = make_client(traces={})
465483
client.shutdown()
@@ -649,6 +667,22 @@ def test_a_forked_child_does_not_inherit_the_active_span(self):
649667
assert client.get_active_span() is None
650668
client.shutdown()
651669

670+
def test_an_inherited_span_exiting_in_the_child_does_not_restore_its_parent(
671+
self,
672+
):
673+
client = make_client(traces={})
674+
with client.start_span("outer") as outer:
675+
with client.start_span("inner"):
676+
client._reinit_after_fork()
677+
assert client.get_active_span() is None
678+
child = client.start_span("child")
679+
child.end()
680+
(record,) = client._traces._exporter._queue
681+
assert record.name == "child"
682+
assert record.parent_span_id is None
683+
assert record.trace_id != outer.traceparent().split("-")[1]
684+
client.shutdown()
685+
652686

653687
class TestModuleLevelApi:
654688
def _with_module_client(self, traces, body):

posthog/tracing/_pipeline.py

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -114,10 +114,12 @@ def warn_if_queued(self) -> None:
114114
self._exporter.warn_if_queued()
115115
self._drops.warn_if_due(force=True)
116116

117-
def reinit_after_fork(self) -> None:
117+
def reinit_after_fork(self, active_var: Optional[ContextVar] = None) -> None:
118118
# Runs in the forked child before user code; the parent's spans stay
119119
# with the parent.
120120
self._lock = threading.Lock()
121+
if active_var is not None:
122+
self._active_var = active_var
121123
self._live_spans.clear()
122124
self._drops.reinit_after_fork()
123125
self._exporter.reinit_after_fork()

0 commit comments

Comments
 (0)