From ad276552f7b0a849ceea67d7f64a373755222ed5 Mon Sep 17 00:00:00 2001 From: Eli Reisman Date: Fri, 23 Oct 2026 17:26:05 -0700 Subject: [PATCH] fix: fill opentelemetry trace ids below context tags and super properties The $trace_id and $span_id taken from the active OpenTelemetry span are values the SDK adds, so they now fill after the call's properties, context tags and super_properties, and before before_send. They fill as a pair: when any layer sets either key, the SDK adds neither. This covers capture, capture_ai and capture_exception on Client and AsyncPosthog. Co-Authored-By: Claude Opus 5.5 --- .sampo/changesets/trace-ids-as-sdk-values.md | 5 + docs/migration-7.x-to-8.0.md | 3 +- posthog/__init__.py | 8 +- posthog/args.py | 3 + posthog/async_client.py | 9 +- posthog/capture_event.py | 16 ++- posthog/client.py | 21 +++- posthog/test/test_event_options.py | 114 +++++++++++++++++++ 8 files changed, 165 insertions(+), 14 deletions(-) create mode 100644 .sampo/changesets/trace-ids-as-sdk-values.md diff --git a/.sampo/changesets/trace-ids-as-sdk-values.md b/.sampo/changesets/trace-ids-as-sdk-values.md new file mode 100644 index 000000000..f103af11e --- /dev/null +++ b/.sampo/changesets/trace-ids-as-sdk-values.md @@ -0,0 +1,5 @@ +--- +pypi/posthog: major +--- + +The `$trace_id` and `$span_id` that the SDK takes from the active OpenTelemetry span now fill after the call's `properties`, context tags and `super_properties`, and before `before_send`. A `$trace_id` or `$span_id` in a context tag or `super_properties` now replaces the span's IDs, and setting either key stops the SDK adding both. `AsyncPosthog` follows the same rules. diff --git a/docs/migration-7.x-to-8.0.md b/docs/migration-7.x-to-8.0.md index a0f3314e7..da9aacc49 100644 --- a/docs/migration-7.x-to-8.0.md +++ b/docs/migration-7.x-to-8.0.md @@ -191,7 +191,7 @@ Values apply in this order. Steps 2 to 4 fill only the options and properties th 1. the `options` and `properties` of the call, then the call's `disable_geoip` argument, which fills `$geoip_disable` 2. context options and tags 3. `super_options` and `super_properties` -4. values the SDK sets: `$is_server` from `is_server`, `$geoip_disable` from the client's `disable_geoip` setting, system properties such as `$os` and `$python_version`, the `$feature/` and `$active_feature_flags` properties from `flags` or `send_feature_flags`, `options.process_person_profile = false` for events without a distinct ID, and `$release_id` from `POSTHOG_RELEASE_ID` +4. values the SDK sets: `$is_server` from `is_server`, `$geoip_disable` from the client's `disable_geoip` setting, system properties such as `$os` and `$python_version`, the `$feature/` and `$active_feature_flags` properties from `flags` or `send_feature_flags`, `$trace_id` and `$span_id` from the active OpenTelemetry span, `options.process_person_profile = false` for events without a distinct ID, and `$release_id` from `POSTHOG_RELEASE_ID` 5. `before_send`, which sees the result of steps 1 to 4 and can change or remove any of it 6. legacy properties, which fill unset options and are then removed @@ -205,6 +205,7 @@ These changes follow from this order: - Properties passed to a call now override `super_properties`. `super_properties` can no longer change `$lib` or `$lib_version`. - A `$is_server`, `$geoip_disable` or system property such as `$os` that you set in a call or a context tag now wins over the SDK's value. In 7.x the SDK overwrote it. `super_properties` win over the SDK's value too, so `super_properties={"$geoip_disable": False}` turns GeoIP lookup on for events, even with `disable_geoip=True`. - Feature flag properties from `flags` or `send_feature_flags` still lose to the same key in the call's `properties`, a context tag or `super_properties`, as in 7.x. +- A `$trace_id` or `$span_id` in a context tag or `super_properties` now replaces the active OpenTelemetry span's IDs. In 7.x the span's IDs beat context tags. Setting either key in the call's `properties`, a context tag or `super_properties` stops the SDK adding both. In 7.x the SDK still added the other key. - A `disable_geoip` argument to a call belongs to that event, so it wins over context tags and `super_properties`. A `$geoip_disable` in the call's own `properties` still wins over it. `disable_geoip=False` now sends `$geoip_disable: false`. In 7.x it sent no `$geoip_disable` property. - The `groups` argument merges into a `$groups` property of the call, and wins key by key. In 7.x it replaced the property. MCP events merge their identity's groups into a custom `$groups` property the same way. - A `$set`, `$set_once`, `$groups` or `$group_set` in `super_properties` no longer replaces the whole value of the call. The two merge, and the call wins key by key. diff --git a/posthog/__init__.py b/posthog/__init__.py index b5bea2600..d97d79fc8 100644 --- a/posthog/__init__.py +++ b/posthog/__init__.py @@ -432,9 +432,11 @@ def get_context_options() -> Dict[str, Any]: capture_trace_context: When OpenTelemetry is installed and a valid span is active at capture time, add its trace and span IDs as ``$trace_id`` and ``$span_id`` properties to events captured with ``capture()`` and - ``capture_ai()``. Explicit ``$trace_id``/``$span_id`` values passed in - ``properties`` win. Exception events always attach these IDs regardless - of this setting. Defaults to False. + ``capture_ai()``. They fill after context tags and ``super_properties``, + before ``before_send``. A ``$trace_id`` or ``$span_id`` from the event, a + context tag or ``super_properties`` stops the SDK adding either ID. + Exception events always attach these IDs regardless of this setting. + Defaults to False. code_variables_mask_patterns: Variable-name patterns to mask when capturing code variables. code_variables_ignore_patterns: Variable-name patterns to omit when capturing diff --git a/posthog/args.py b/posthog/args.py index 1b2aa4118..9822c333f 100644 --- a/posthog/args.py +++ b/posthog/args.py @@ -69,6 +69,9 @@ class OptionalCaptureArgs(TypedDict): _property_allowlist: NotRequired[ Optional[FrozenSet[str]] ] # Internal: strict allowlist applied to the fully-enriched event properties. Used by minimal $feature_flag_called events. + _capture_trace_context: NotRequired[ + bool + ] # Internal: add the active OpenTelemetry span's ids even when capture_trace_context is off. Used by capture_exception. class OptionalSetArgs(TypedDict): diff --git a/posthog/async_client.py b/posthog/async_client.py index be365b83c..5a7f849f5 100644 --- a/posthog/async_client.py +++ b/posthog/async_client.py @@ -547,6 +547,7 @@ def _event_defaults( disable_geoip: Optional[bool] = None, system_properties: Optional[dict[str, Any]] = None, flag_properties: Optional[dict[str, Any]] = None, + trace_properties: Optional[dict[str, Any]] = None, ) -> _EventDefaults: return _build_event_defaults( super_properties=self.super_properties, @@ -561,14 +562,13 @@ def _event_defaults( call_disable_geoip=disable_geoip, system_properties=system_properties, flag_properties=flag_properties, + trace_properties=trace_properties, ) def _build_capture_event( self, event: str, kwargs: OptionalCaptureArgs ) -> tuple[dict[str, Any], Any, _EventDefaults]: properties = dict(kwargs.get("properties") or {}) - if self.capture_trace_context: - properties = {**_get_current_otel_span_properties(), **properties} properties = _add_context_session_id(properties) distinct_id, personless = _get_identity_state(kwargs.get("distinct_id")) @@ -607,6 +607,11 @@ def _build_capture_event( disable_geoip=kwargs.get("disable_geoip"), system_properties=system_context(), flag_properties=flag_properties, + trace_properties=( + _get_current_otel_span_properties() + if self.capture_trace_context + else None + ), ), ) diff --git a/posthog/capture_event.py b/posthog/capture_event.py index b99061e89..36c5aac0c 100644 --- a/posthog/capture_event.py +++ b/posthog/capture_event.py @@ -133,6 +133,7 @@ class _EventDefaults: property_layers: tuple[Mapping[str, Any], ...] = () option_layers: tuple[Mapping[str, Any], ...] = () property_allowlist: Optional[Collection[str]] = None + trace_properties: Optional[Mapping[str, Any]] = None def _build_event_defaults( @@ -149,6 +150,7 @@ def _build_event_defaults( call_disable_geoip: Optional[bool] = None, system_properties: Optional[Mapping[str, Any]] = None, flag_properties: Optional[Mapping[str, Any]] = None, + trace_properties: Optional[Mapping[str, Any]] = None, ) -> _EventDefaults: """Order the layers: per-call arguments, context, global, then SDK values. @@ -157,7 +159,8 @@ def _build_event_defaults( event's own ``$geoip_disable`` property beats it. ``flag_properties`` (the ``$feature/*`` and ``$active_feature_flags`` values from ``flags`` or ``send_feature_flags``) are SDK values, so context and super properties - beat them. + beat them. ``trace_properties`` (``$trace_id`` and ``$span_id`` from the + active OpenTelemetry span) are SDK values that fill as a pair. """ call_properties = ( {} if call_disable_geoip is None else {"$geoip_disable": call_disable_geoip} @@ -184,6 +187,7 @@ def _build_event_defaults( derived_options or {}, ), property_allowlist=property_allowlist, + trace_properties=trace_properties, ) @@ -195,7 +199,8 @@ def _fill_event_defaults( A property is unset only when its key is missing. An option is unset when it is missing or ``None``, the same rule hoisting uses. ``$set``, ``$set_once``, ``$groups`` and ``$group_set`` fill one level deep when the - event's value and the default are both dicts. + event's value and the default are both dicts. ``trace_properties`` fill + last, and only when the event and every layer leave all their keys unset. """ if defaults is None: return @@ -218,6 +223,13 @@ def _fill_event_defaults( and isinstance(value, Mapping) ): properties[key] = {**_clean(dict(value)), **existing} + # A trace id and a span id from different spans would point at no real span, + # so a value for either key from any layer keeps out both of the SDK's ids. + trace_properties = defaults.trace_properties or {} + if not any(key in properties for key in trace_properties): + for key, value in trace_properties.items(): + if allowlist is None or key in allowlist: + properties[key] = _clean(value) options = _event_options(msg.get("options")) for option_layer in defaults.option_layers: for key, value in option_layer.items(): diff --git a/posthog/client.py b/posthog/client.py index c0393738d..715776a44 100644 --- a/posthog/client.py +++ b/posthog/client.py @@ -882,8 +882,10 @@ def __init__( capture_trace_context: When OpenTelemetry is installed and a valid span is active at capture time, add its trace and span IDs as ``$trace_id`` and ``$span_id`` properties to events captured with ``capture()`` and - ``capture_ai()``, so they can be correlated with backend traces. Explicit - ``$trace_id``/``$span_id`` values passed in ``properties`` win. Exception + ``capture_ai()``, so they can be correlated with backend traces. They fill + after context tags and ``super_properties``, before ``before_send``. A + ``$trace_id`` or ``$span_id`` from the event, a context tag or + ``super_properties`` stops the SDK adding either ID. Exception events (``capture_exception``) always attach these IDs regardless of this setting. Defaults to False. traces: Config dict for distributed tracing: ``service_name``, @@ -1806,8 +1808,11 @@ def _capture( properties = dict(properties or {}) - if self.capture_trace_context: - properties = {**_get_current_otel_span_properties(), **properties} + trace_properties = ( + _get_current_otel_span_properties() + if self.capture_trace_context or kwargs.get("_capture_trace_context") + else None + ) properties = _add_context_session_id(properties) @@ -1922,6 +1927,7 @@ def _capture( derived_options=_personless_options(personless), system_properties=system_context(), flag_properties=extra_properties, + trace_properties=trace_properties, ) def _parse_send_feature_flags(self, send_feature_flags) -> SendFeatureFlagsOptions: @@ -2237,7 +2243,8 @@ def capture_exception( Capture an exception for error tracking. When OpenTelemetry is installed and a valid span is active, its trace and - span IDs are added as ``$trace_id`` and ``$span_id`` event properties. + span IDs are added as ``$trace_id`` and ``$span_id`` event properties, + unless the event, a context tag or ``super_properties`` sets either one. Args: exception: The exception to capture. @@ -2314,7 +2321,6 @@ def capture_exception( caller_properties = properties properties = { - **_get_current_otel_span_properties(), "$exception_list": all_exceptions_with_trace_and_in_app, "$exception_level": _exception_level( capture_metadata.get("level"), level @@ -2386,6 +2392,7 @@ def capture_exception( send_feature_flags=send_feature_flags, disable_geoip=disable_geoip, options=kwargs.get("options", None), + _capture_trace_context=True, ) # Mark the exception as captured to prevent duplicate captures @@ -2537,6 +2544,7 @@ def _enqueue( derived_options=None, system_properties=None, flag_properties=None, + trace_properties=None, ): # type: (...) -> Optional[str] """Push a new `msg` onto a lane's queue (analytics when unspecified), return the event uuid or None.""" @@ -2585,6 +2593,7 @@ def _enqueue( call_disable_geoip=disable_geoip, system_properties=system_properties, flag_properties=flag_properties, + trace_properties=trace_properties, ), ) diff --git a/posthog/test/test_event_options.py b/posthog/test/test_event_options.py index ab28f3788..34e4b9d42 100644 --- a/posthog/test/test_event_options.py +++ b/posthog/test/test_event_options.py @@ -2,6 +2,7 @@ import logging import pytest +from opentelemetry.trace import NonRecordingSpan, SpanContext, TraceFlags, use_span from posthog import AsyncPosthog, new_context, set_context_option, tag from posthog.ai.utils import _capture_ai_event @@ -475,6 +476,119 @@ async def test_async_caller_layers_beat_flag_properties(source): _assert_flag_layers(events, source) +SPAN_IDS = { + "$trace_id": "00000000000000000000000000000123", + "$span_id": "0000000000000456", +} + +# Each caller layer sets one key of the pair, so the SDK adds neither span id. +TRACE_ID_CASES = [ + pytest.param(None, SPAN_IDS, id="no_layer"), + pytest.param("event", {"$trace_id": "caller"}, id="event_trace_id"), + pytest.param("context", {"$trace_id": "caller"}, id="context_trace_id"), + pytest.param("super", {"$span_id": "caller"}, id="super_span_id"), +] + +TRACE_CALLS = { + "capture": lambda c, p: c.capture("e", distinct_id="u", properties=p), + "capture_ai": lambda c, p: c.capture_ai( + "$ai_generation", distinct_id="u", properties=p + ), + "capture_exception": lambda c, p: c.capture_exception( + ValueError("boom"), distinct_id="u", properties=p + ), +} + + +def _capture_in_span(method, source=None): + span = NonRecordingSpan( + SpanContext( + trace_id=0x123, + span_id=0x456, + is_remote=False, + trace_flags=TraceFlags.SAMPLED, + ) + ) + + def call(client): + with new_context(fresh=True), use_span(span): + if source == "context": + tag("$trace_id", "caller") + properties = {"$trace_id": "caller"} if source == "event" else None + return TRACE_CALLS[method](client, properties) + + return call + + +def _trace_config(source, capture_trace_context=True): + config: dict = {"capture_trace_context": capture_trace_context} + if source == "super": + config["super_properties"] = {"$span_id": "caller"} + return config + + +def _span_ids(properties): + return {k: properties[k] for k in SPAN_IDS if k in properties} + + +def _sync_trace_context(method): + # Sync capture_exception adds the span ids even with capture_trace_context off. + return method != "capture_exception" + + +@pytest.mark.parametrize("method", list(TRACE_CALLS)) +@pytest.mark.parametrize("source,expected", TRACE_ID_CASES) +def test_sync_span_ids_fill_as_a_pair_below_caller_layers(method, source, expected): + events = _sync_wire_events( + _capture_in_span(method, source), + **_trace_config(source, _sync_trace_context(method)), + ) + assert _span_ids(events[0]["properties"]) == expected + + +@pytest.mark.asyncio +@pytest.mark.parametrize("method", list(TRACE_CALLS)) +@pytest.mark.parametrize("source,expected", TRACE_ID_CASES) +async def test_async_span_ids_fill_as_a_pair_below_caller_layers( + method, source, expected +): + events = await _async_wire_events( + _capture_in_span(method, source), **_trace_config(source) + ) + assert _span_ids(events[0]["properties"]) == expected + + +def _span_id_hook(seen): + def hook(msg): + seen.append(_span_ids(msg["properties"])) + return {**msg, "properties": {**msg["properties"], "$span_id": "hook"}} + + return hook + + +@pytest.mark.parametrize("method", list(TRACE_CALLS)) +def test_sync_before_send_sees_and_changes_span_ids(method): + seen: list = [] + events = _sync_wire_events( + _capture_in_span(method), + before_send=_span_id_hook(seen), + **_trace_config(None, _sync_trace_context(method)), + ) + assert seen == [SPAN_IDS] + assert _span_ids(events[0]["properties"]) == {**SPAN_IDS, "$span_id": "hook"} + + +@pytest.mark.asyncio +@pytest.mark.parametrize("method", list(TRACE_CALLS)) +async def test_async_before_send_sees_and_changes_span_ids(method): + seen: list = [] + events = await _async_wire_events( + _capture_in_span(method), before_send=_span_id_hook(seen), **_trace_config(None) + ) + assert seen == [SPAN_IDS] + assert _span_ids(events[0]["properties"]) == {**SPAN_IDS, "$span_id": "hook"} + + GEOIP_CALL_CASES = [ pytest.param(True, "context", False, True, id="call_beats_context"), pytest.param(True, "super", False, True, id="call_beats_super"),