diff --git a/.sampo/changesets/identity-and-context-rules.md b/.sampo/changesets/identity-and-context-rules.md new file mode 100644 index 00000000..affd35e8 --- /dev/null +++ b/.sampo/changesets/identity-and-context-rules.md @@ -0,0 +1,5 @@ +--- +pypi/posthog: major +--- + +`group_identify` falls back to the context distinct ID and then to `$_` instead of a random ID, and no longer adds the context session ID. `alias()` no longer takes its target from the context distinct ID. Context tags and options now apply to `alias` and `group_identify`. An empty or whitespace-only `distinct_id` counts as missing, and generated personless distinct IDs are UUIDv7. `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 f4c21e5d..fdf51725 100644 --- a/docs/migration-7.x-to-8.0.md +++ b/docs/migration-7.x-to-8.0.md @@ -25,6 +25,7 @@ You need to change code if your app does any of these: - sets `$lib` or `$lib_version`, or filters on `$lib = "posthog-python-mcp"` - passes a personal API key (`phx_...`) as the project API key - reads event content from the SDK's debug log +- calls `alias()` without a `distinct_id` and relies on the context distinct ID ## Endpoints @@ -127,6 +128,15 @@ The SDK moves them out of `properties` for you. - `None` counts as unset and is removed. - Any other value is removed and not sent, because capture would reject the whole batch. 7.x sent it as a property. The SDK logs a warning for each one, with the key and the value's type but not the value. +## Identity and request context + +- An empty or whitespace-only `distinct_id` counts as missing, so the event gets a generated personless distinct ID. 7.x sent a whitespace-only ID, and capture dropped the event. +- Generated personless distinct IDs are UUIDv7. 7.x generated UUIDv4. +- `group_identify` uses the `distinct_id` you pass, then the context distinct ID, then `$_`. 7.x fell back to a random ID, which created a new person on every call. +- `group_identify` no longer adds the context session ID. A session takes the person of its latest event, and this event's distinct ID is not the session's user. +- `alias()` needs an explicit `distinct_id`. It no longer falls back to the context distinct ID, which can come from a request header the client controls. Without one, the call is dropped with a warning. +- Context tags and context options now apply to `alias` and `group_identify`, below the call's own values, as they do for `capture`. + ## Exceptions - `capture_exception` takes a `level=` argument on `Client`, `AsyncPosthog` and the module. It sets `$exception_level`, for example `"warning"` or `"fatal"`. The default is `"error"`, and an unknown value counts as unset. diff --git a/posthog/async_client.py b/posthog/async_client.py index bd738df6..c9b3cd23 100644 --- a/posthog/async_client.py +++ b/posthog/async_client.py @@ -55,6 +55,8 @@ _parse_flag_payload, _add_context_session_id, _context_tag_defaults, + _explicit_distinct_id, + _group_identify_distinct_id, _personless_options, _positive_config_value, add_context_tags as _add_context_tags, @@ -798,7 +800,9 @@ def group_identify( ) return None - resolved_distinct_id = _get_identity_state(distinct_id)[0] + resolved_distinct_id = _group_identify_distinct_id( + distinct_id, group_type, group_key + ) msg: dict[str, Any] = { "event": "$groupidentify", "properties": { @@ -811,10 +815,14 @@ def group_identify( "uuid": uuid, "options": _event_options(options), } - session_id = _get_context_session_id() - if session_id: - msg["properties"]["$session_id"] = str(session_id) - return self._enqueue_built_event(msg, disable_geoip) + # No context session: this event's distinct id is not the session's + # user, and a session takes the person of its latest event. + return self._enqueue_built_event( + msg, + disable_geoip, + _get_context_options(), + context_properties=_context_tag_defaults(), + ) except Exception as error: if self.debug: raise @@ -835,8 +843,10 @@ def alias( if not resolved_previous_id: self.log.warning("alias() called without a previous_id, dropping event") return None - resolved_distinct_id, personless = _get_identity_state(distinct_id) - if personless: + # The alias target always comes from the caller: a context id can + # come from a client-controlled request header. + resolved_distinct_id = _explicit_distinct_id(distinct_id) + if not resolved_distinct_id: self.log.warning("alias() called without a distinct_id, dropping event") return None msg: dict[str, Any] = { @@ -853,7 +863,12 @@ def alias( session_id = _get_context_session_id() if session_id: msg["properties"]["$session_id"] = str(session_id) - return self._enqueue_built_event(msg, disable_geoip) + return self._enqueue_built_event( + msg, + disable_geoip, + _get_context_options(), + context_properties=_context_tag_defaults(), + ) except Exception as error: if self.debug: raise @@ -865,6 +880,7 @@ def _enqueue_built_event( msg: dict[str, Any], disable_geoip: Optional[bool], context_options: Optional[dict[str, Any]] = None, + context_properties: Optional[dict[str, Any]] = None, ) -> Optional[str]: prepared, sent_uuid = self._prepare_event(msg) if prepared is None or sent_uuid is None: @@ -872,7 +888,9 @@ def _enqueue_built_event( if not self.send: return sent_uuid defaults = self._event_defaults( - context_options=context_options, disable_geoip=disable_geoip + context_properties=context_properties, + context_options=context_options, + disable_geoip=disable_geoip, ) if not self._enqueue_prepared_event(prepared, self._analytics_lane, defaults): return None diff --git a/posthog/client.py b/posthog/client.py index f0ffa718..95d96c8b 100644 --- a/posthog/client.py +++ b/posthog/client.py @@ -132,6 +132,7 @@ to_values, ) from posthog.utils import ( + _uuid7, FlagCache, RedisFlagCache, SizeLimitedDict, @@ -262,17 +263,37 @@ def _positive_config_value( return value +def _explicit_distinct_id(passed) -> Optional[str]: + """The caller's distinct id, or None when it is missing, empty or whitespace.""" + stringified = stringify_id(passed) + if stringified and stringified.strip(): + return stringified + return None + + def get_identity_state(passed) -> tuple[str, bool]: """Returns the distinct id to use, and whether this is a personless event or not""" - stringified = stringify_id(passed) - if stringified and len(stringified): - return (stringified, False) + explicit = _explicit_distinct_id(passed) + if explicit: + return (explicit, False) context_id = get_context_distinct_id() if context_id: return (context_id, False) - return (str(uuid4()), True) + return (str(_uuid7()), True) + + +def _group_identify_distinct_id(passed, group_type, group_key) -> str: + """The caller's id, else the context id, else one stable id per group. + + Ingestion creates a person for each new distinct id, so a random id + would add a person on every call. + """ + distinct_id, personless = get_identity_state(passed) + if personless: + return f"${group_type}_{group_key}" + return distinct_id def _personless_options(personless: bool) -> dict[str, Any]: @@ -2076,6 +2097,8 @@ def group_identify( ignored and replaced with a newly generated UUID. disable_geoip: Whether to disable GeoIP for this event. distinct_id: The distinct ID of the user performing the action. + Falls back to the context distinct ID, then to + ``$_``. options: Capture options for this event, sent as given. Examples: @@ -2105,8 +2128,7 @@ def group_identify( properties = properties or {} - # group_identify is purposefully always personful - distinct_id = get_identity_state(distinct_id)[0] + distinct_id = _group_identify_distinct_id(distinct_id, group_type, group_key) msg: Dict[str, Any] = { "event": "$groupidentify", @@ -2121,11 +2143,14 @@ def group_identify( "options": _event_options(options), } - # NOTE - group_identify doesn't generally use context properties - should it? - if get_context_session_id(): - msg["properties"]["$session_id"] = str(get_context_session_id()) - - return self._enqueue(msg, disable_geoip) + # No context session: this event's distinct id is not the session's + # user, and a session takes the person of its latest event. + return self._enqueue( + msg, + disable_geoip, + context_properties=_context_tag_defaults(), + context_options=_context_get_context_options(), + ) @no_throw() def alias( @@ -2143,9 +2168,9 @@ def alias( Args: previous_id: The previous distinct ID. Required - the call is dropped with a warning if it is missing or empty. - distinct_id: The new distinct ID to alias to. Falls back to the - context distinct ID; the call is dropped with a warning if - neither is available. + distinct_id: The new distinct ID to alias to. Required - the call is + dropped with a warning if it is missing or empty. The context + distinct ID is never used here. timestamp: The timestamp of the event. UTC is preferred; non-UTC datetimes and parseable ISO timestamp strings are converted to UTC. uuid: A unique identifier for the event. If provided, it must be a @@ -2171,10 +2196,10 @@ def alias( ) return None - (distinct_id, personless) = get_identity_state(distinct_id) - - if personless: - # No alias target was passed and none is available from context. + # The alias target always comes from the caller: a context id can come + # from a client-controlled request header. + distinct_id = _explicit_distinct_id(distinct_id) + if not distinct_id: self.log.warning( "alias() called without a distinct_id, dropping the $create_alias event" ) @@ -2195,7 +2220,12 @@ def alias( if get_context_session_id(): msg["properties"]["$session_id"] = str(get_context_session_id()) - return self._enqueue(msg, disable_geoip) + return self._enqueue( + msg, + disable_geoip, + context_properties=_context_tag_defaults(), + context_options=_context_get_context_options(), + ) def capture_exception( self, diff --git a/posthog/test/test_async_client.py b/posthog/test/test_async_client.py index f9ae5165..515cdf0c 100644 --- a/posthog/test/test_async_client.py +++ b/posthog/test/test_async_client.py @@ -15,7 +15,11 @@ from posthog.capture_send import _CAPTURE_AI_V1_PATH, _CAPTURE_V1_PATH from posthog.consumer import AI_MAX_MSG_SIZE, AI_MAX_PROPERTIES_SIZE, MAX_MSG_SIZE from posthog.contexts import ( + identify_context, new_context, + set_context_option, + set_context_session, + tag, set_capture_exception_code_variables_context, set_code_variables_detect_secrets_context, set_code_variables_ignore_patterns_context, @@ -483,6 +487,50 @@ async def send_batch(api_key, host, batch, **kwargs): assert batches[0][0]["event"] == expected_event +@pytest.mark.asyncio +@pytest.mark.parametrize( + ("method_name", "method_kwargs", "expected_distinct_id", "expected_session"), + [ + ( + "group_identify", + {"group_type": "company", "group_key": "acme-1"}, + "context-user", + None, + ), + ( + "alias", + {"previous_id": "anon-1", "distinct_id": "user-1"}, + "anon-1", + "context-session", + ), + ], +) +async def test_identity_methods_take_context_values_by_the_context_rule( + method_name, method_kwargs, expected_distinct_id, expected_session +): + batches = [] + + async def send_batch(api_key, host, batch, **kwargs): + batches.append(batch) + + with patch_async_capture_send(side_effect=send_batch): + async with AsyncPosthog("test-key", flush_interval=30) as client: + with new_context(fresh=True): + identify_context("context-user") + set_context_session("context-session") + tag("tenant", "acme") + set_context_option("product_tour_id", "tour-1") + assert getattr(client, method_name)(**method_kwargs) is not None + assert client.alias("anon-2", None) is None + await client.flush(timeout_seconds=1) + + msg = batches[0][0] + assert msg["distinct_id"] == expected_distinct_id + assert msg["properties"].get("$session_id") == expected_session + assert msg["properties"]["tenant"] == "acme" + assert msg["options"]["product_tour_id"] == "tour-1" + + @pytest.mark.asyncio async def test_capture_after_shutdown_is_dropped_without_restarting_workers(): with mock.patch("posthog.async_client._build_client") as build_client: diff --git a/posthog/test/test_client.py b/posthog/test/test_client.py index 25ecab95..8ab03b4f 100644 --- a/posthog/test/test_client.py +++ b/posthog/test/test_client.py @@ -20,14 +20,20 @@ from posthog.capture_compression import CaptureCompression from posthog.capture_send import _CAPTURE_AI_V1_PATH, _CAPTURE_V1_PATH from posthog.client import Client -from posthog.contexts import get_context_session_id, new_context, set_context_session +from posthog.contexts import ( + get_context_session_id, + identify_context, + new_context, + set_context_option, + set_context_session, +) from posthog.request import APIError, GetResponse from posthog.test.logging_helpers import capture_message_only_logs from posthog.test.test_utils import FAKE_TEST_API_KEY from posthog.types import FeatureFlag, FeatureFlagResult, LegacyFlagMetadata from posthog.version import VERSION from posthog.contexts import tag -from posthog.test.capture_helpers import patch_capture_send, sent_batch +from posthog.test.capture_helpers import patch_capture_send, sent_batch, sent_events # Legacy single-flag behavior remains covered here; warning emission itself is @@ -2243,6 +2249,83 @@ def test_alias_accepts_non_string_previous_id(self): self.assertEqual(msg["distinct_id"], "0") self.assertEqual(msg["properties"]["distinct_id"], "0") + def _capture_in_request_context(self, call): + with patch_capture_send("client") as mock_post: + client = Client(FAKE_TEST_API_KEY, on_error=self.set_fail, sync_mode=True) + with new_context(fresh=True): + identify_context("context-user") + set_context_session("context-session") + tag("tenant", "acme") + set_context_option("product_tour_id", "tour-1") + result = call(client) + return result, (sent_batch(mock_post)[0] if mock_post.called else None) + + @parameterized.expand( + [ + ("caller_id", "caller-user", "caller-user"), + ("whitespace_caller_id", " ", "context-user"), + ] + ) + def test_group_identify_takes_context_values_but_not_its_session( + self, _, distinct_id, expected_distinct_id + ): + _, msg = self._capture_in_request_context( + lambda client: client.group_identify( + "company", "acme-1", distinct_id=distinct_id + ) + ) + + self.assertEqual(msg["distinct_id"], expected_distinct_id) + self.assertNotIn("$session_id", msg["properties"]) + self.assertEqual(msg["properties"]["tenant"], "acme") + self.assertEqual(msg["options"]["product_tour_id"], "tour-1") + + def test_group_identify_without_any_distinct_id_uses_the_group_id(self): + with patch_capture_send("client") as mock_post: + client = Client(FAKE_TEST_API_KEY, on_error=self.set_fail, sync_mode=True) + client.group_identify("company", "acme-1") + client.group_identify("company", "acme-1") + + self.assertEqual( + [msg["distinct_id"] for msg in sent_events(mock_post)], + ["$company_acme-1", "$company_acme-1"], + ) + + @parameterized.expand( + [ + ("caller_target", "user-1", "user-1"), + ("context_only_target", None, None), + ("whitespace_target", " ", None), + ] + ) + def test_alias_takes_its_target_only_from_the_caller( + self, _, distinct_id, expected_alias + ): + result, msg = self._capture_in_request_context( + lambda client: client.alias("anon-1", distinct_id) + ) + + if expected_alias is None: + self.assertIsNone(result) + self.assertIsNone(msg) + return + self.assertEqual(msg["properties"]["alias"], expected_alias) + self.assertEqual(msg["properties"]["$session_id"], "context-session") + self.assertEqual(msg["properties"]["tenant"], "acme") + self.assertEqual(msg["options"]["product_tour_id"], "tour-1") + + @parameterized.expand([("none", None), ("empty", ""), ("whitespace", " \t ")]) + def test_capture_without_a_distinct_id_sends_a_personless_v7_id( + self, _, distinct_id + ): + with patch_capture_send("client") as mock_post: + client = Client(FAKE_TEST_API_KEY, on_error=self.set_fail, sync_mode=True) + client.capture("event", distinct_id=distinct_id) + + msg = sent_batch(mock_post)[0] + self.assertEqual(UUID(msg["distinct_id"]).version, 7) + self.assertEqual(msg["options"]["process_person_profile"], False) + def test_alias_without_distinct_id_is_dropped(self): with patch_capture_send("client") as mock_post: client = Client(FAKE_TEST_API_KEY, on_error=self.set_fail, sync_mode=True)