Skip to content

Commit f2bba20

Browse files
committed
fix(traces): keep the module-level setup from turning an explicit traces config off
setup() runs on every module-level call and re-applied posthog.traces whenever no pipeline existed yet, so a default client built with its own traces config lost it on the first capture(), and a failed init was retried and re-logged on every call. The module option now applies only where the client has none, and a failed init latches as False. Shutdown unregisters the sync-mode exit drain so the client is collectable, the flush docstring states its worst case, Span is re-exported for pyright strict and checked in CI, the fork ContextVar is required, and the start_span docstring says what a forked child inherits.
1 parent c286af7 commit f2bba20

7 files changed

Lines changed: 60 additions & 12 deletions

File tree

.github/scripts/check_strict_types.sh

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -26,8 +26,11 @@ all_flags: dict[str, FlagValue] | None = posthog.get_all_flags("user", groups=gr
2626
enabled: bool | None = posthog.feature_enabled("flag", "user", groups=groups)
2727
payload: object | None = client.get_feature_flag_payload("flag", "user", groups=groups)
2828
evaluations: FeatureFlagEvaluations = posthog.evaluate_flags(123, groups=groups)
29+
span: posthog.Span = client.start_span("job")
30+
active: posthog.Span | None = posthog.get_active_span()
31+
span.end()
2932
30-
_ = (flag_value, all_flags, enabled, payload, evaluations)
33+
_ = (flag_value, all_flags, enabled, payload, evaluations, active)
3134
PY
3235

3336
"$tmp/.venv/bin/python" - <<'PY' > "$tmp/public_api_access.py"

posthog/__init__.py

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,7 @@
1212
from posthog.capture_compression import CaptureCompression as CaptureCompression
1313
from posthog.capture_mode import CaptureMode as CaptureMode
1414
from posthog.client import Client
15-
from posthog.tracing.span import Span
15+
from posthog.tracing.span import Span as Span
1616
from posthog.async_client import AsyncClient as AsyncClient
1717
from posthog.async_client import AsyncPosthog as AsyncPosthog
1818
from posthog.exception_capture import ExceptionCapture
@@ -1393,7 +1393,9 @@ def setup() -> Client:
13931393
# already forced setup()) still applies until the metrics API is first used.
13941394
if default_client._metrics is None:
13951395
default_client._metrics_config = metrics
1396-
if default_client._traces is None:
1396+
# traces=None means off, so the module option applies only where the client
1397+
# has none; False is the latch of an init that failed and stays off.
1398+
if traces is not None and default_client._traces_config is None:
13971399
default_client._traces_config = traces
13981400

13991401
return default_client

posthog/client.py

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2539,7 +2539,7 @@ def _traces_pipeline(self) -> Optional[PostHogTraces]:
25392539
# Off rather than defaults: defaults would drop a
25402540
# before_span_send hook and export unscrubbed spans.
25412541
self.log.exception("Error initializing traces; tracing is off")
2542-
self._traces_config = None
2542+
self._traces_config = False
25432543
return self._traces
25442544

25452545
def _tracing_context(self) -> Dict[str, Optional[str]]:
@@ -2578,6 +2578,8 @@ def start_span(
25782578
attributes: Initial attributes.
25792579
parent: A span handle, or an inbound W3C ``traceparent`` header
25802580
value to continue a remote trace. Defaults to the active span.
2581+
A forked child starts with no active span; pass the parent
2582+
span to continue a trace across a fork.
25812583
tracestate: The inbound ``tracestate`` header accompanying a
25822584
``traceparent`` string ``parent``; preserved and propagated.
25832585
start_time: A ``datetime`` or epoch seconds, to backdate the span.
@@ -2638,7 +2640,9 @@ def flush(self, timeout_seconds: Optional[float] = 10) -> None:
26382640
Queued spans are sent at the same time, within the same
26392641
budget: at least one span request is attempted even when the
26402642
budget is already spent, no further one starts once it is, and
2641-
each request is bounded by ``timeout``.
2643+
each request is bounded by ``timeout``. The wait for that first
2644+
request is not cut short, so a flush can take up to
2645+
``timeout_seconds`` plus ``timeout`` in the worst case.
26422646
26432647
Examples:
26442648
```python
@@ -2893,6 +2897,8 @@ def _shutdown_once(self, errors: list[Exception]) -> None:
28932897
self._run_lifecycle_cleanup(
28942898
"Failed to close traces on shutdown", traces.close, errors
28952899
)
2900+
# The sync-mode exit drain, so a shut-down client is collectable.
2901+
atexit.unregister(self._atexit_spans)
28962902
self._join_once(errors, flush_queues=False, lanes_prepared=True)
28972903
self._run_lifecycle_cleanup(
28982904
"Failed to clear feature flag deduplication state on shutdown",

posthog/test/tracing/test_client_traces.py

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -125,6 +125,16 @@ def test_a_bad_traces_config_degrades_to_defaults(self):
125125
assert isinstance(client.start_span("x"), RecordingSpan)
126126
client.shutdown()
127127

128+
def test_a_failed_init_is_not_retried_on_the_next_call(self):
129+
client = make_client(traces={})
130+
with mock.patch(
131+
"posthog.client.resolve_traces_config", side_effect=RuntimeError("no")
132+
) as resolve:
133+
assert client.start_span("x") is NOOP_SPAN
134+
assert client.start_span("y") is NOOP_SPAN
135+
assert resolve.call_count == 1
136+
client.shutdown()
137+
128138
def test_never_starts_a_pipeline_on_a_client_without_traces(self):
129139
client = make_client()
130140
client.flush()
@@ -623,6 +633,13 @@ def test_sync_mode_registers_the_span_exit_drain_when_tracing_starts(self):
623633
register.assert_called_once_with(client._atexit_spans)
624634
client.shutdown()
625635

636+
def test_shutdown_unregisters_the_sync_mode_exit_drain(self):
637+
client = make_client(traces={}, sync_mode=True)
638+
client.start_span("x").end()
639+
with mock.patch("posthog.client.atexit.unregister") as unregister:
640+
client.shutdown()
641+
unregister.assert_called_once_with(client._atexit_spans)
642+
626643
@pytest.mark.parametrize("traces", [False, None])
627644
def test_sync_mode_without_tracing_registers_no_exit_hook(self, traces):
628645
with mock.patch("posthog.client.atexit.register") as register:
@@ -735,6 +752,25 @@ def body():
735752

736753
self._with_module_client(None, body)
737754

755+
def test_setup_leaves_an_explicitly_configured_default_client_alone(self):
756+
def body():
757+
posthog.default_client = make_client(traces={"service_name": "explicit"})
758+
posthog.setup()
759+
assert isinstance(posthog.start_span("x"), RecordingSpan)
760+
761+
self._with_module_client(None, body)
762+
763+
def test_setup_does_not_retry_a_traces_init_that_failed(self):
764+
def body():
765+
with mock.patch(
766+
"posthog.client.resolve_traces_config", side_effect=RuntimeError("no")
767+
) as resolve:
768+
assert posthog.start_span("x") is NOOP_SPAN
769+
assert posthog.start_span("y") is NOOP_SPAN
770+
assert resolve.call_count == 1
771+
772+
self._with_module_client({"service_name": "broken"}, body)
773+
738774
def test_module_start_span_is_inert_without_config(self):
739775
def body():
740776
assert posthog.start_span("x") is NOOP_SPAN

posthog/test/tracing/test_export.py

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
import threading
2+
from contextvars import ContextVar
23
from types import SimpleNamespace
34
from unittest import mock
45

@@ -903,7 +904,7 @@ def test_a_forked_child_drops_the_inherited_queue_and_timer(self):
903904
pipeline.start_span("parent-span").end()
904905
assert queued(pipeline) and pipeline._exporter._flush_timer is not None
905906
pipeline._exporter._max_export_batch_size = 1
906-
pipeline.reinit_after_fork()
907+
pipeline.reinit_after_fork(ContextVar("child-active", default=None))
907908
assert queued(pipeline) == []
908909
assert pipeline._exporter._flush_timer is None
909910
assert (

posthog/test/tracing/test_pipeline.py

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
import gc
22
import logging
33
import threading
4+
from contextvars import ContextVar
45
import time
56
import warnings
67
import weakref
@@ -582,14 +583,14 @@ def test_close_makes_later_spans_inert_and_closes_the_exporter(self):
582583
def test_a_forked_child_drops_the_parents_live_spans(self):
583584
pipeline, exporter, _ = make()
584585
pipeline.start_span("live")
585-
pipeline.reinit_after_fork()
586+
pipeline.reinit_after_fork(ContextVar("child-active", default=None))
586587
assert pipeline._live_spans == {}
587588
assert exporter.reinitialized
588589

589590
def test_reinit_after_fork_replaces_locks_without_acquiring_them(self):
590591
pipeline, _, _ = make()
591592
pipeline._lock.acquire()
592-
pipeline.reinit_after_fork()
593+
pipeline.reinit_after_fork(ContextVar("child-active", default=None))
593594
assert not pipeline._lock.locked()
594595
pipeline.start_span("a").end()
595596

posthog/tracing/_pipeline.py

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -114,12 +114,11 @@ 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, active_var: Optional[ContextVar] = None) -> None:
117+
def reinit_after_fork(self, active_var: ContextVar) -> None:
118118
# Runs in the forked child before user code; the parent's spans stay
119-
# with the parent.
119+
# with the parent, and the child's active span is the fresh var.
120120
self._lock = threading.Lock()
121-
if active_var is not None:
122-
self._active_var = active_var
121+
self._active_var = active_var
123122
self._live_spans.clear()
124123
self._drops.reinit_after_fork()
125124
self._exporter.reinit_after_fork()

0 commit comments

Comments
 (0)