[FEATURE]Add distinct_id to group_identify (#155)

* [FEATURE]Add distinct_id to group_identify

* [TESTS]Updated test cases for adding distinct_id to group_identify

* [LINT-FIX]client.py and test_client.py

* [CHORE]Verion bump and changelog update

.github/workflows/ci.yml

@@ -18,7 +18,7 @@ jobs:
               with:
                   python-version: 3.8
 
-            - uses: actions/cache@v1
+            - uses: actions/cache@v3
               with:
                   path: ~/.cache/pip
                   key: ${{ runner.os }}-pip-${{ hashFiles('setup.py') }}
@@ -47,14 +47,14 @@ jobs:
         runs-on: ubuntu-latest
 
         steps:
-            - uses: actions/checkout@v1
+            - uses: actions/checkout@v2
               with:
                   fetch-depth: 1
 
-            - name: Set up Python 3.7
-              uses: actions/setup-python@v1
+            - name: Set up Python 3.8
+              uses: actions/setup-python@v2
               with:
-                  python-version: 3.7
+                  python-version: 3.8
 
             - name: Install requirements.txt dependencies with pip
               run: |

.gitignore

@@ -14,4 +14,5 @@ pylint.out
 posthog-analytics
 .idea
 .python-version
-.coverage
+.coverage
+pyrightconfig.json

CHANGELOG.md

@@ -1,3 +1,23 @@
+## 3.7.5 - 2025-01-03
+
+1. Add `distinct_id` to group_identify
+
+## 3.7.4 - 2024-11-25
+
+1. Fix bug where this SDK incorrectly sent feature flag events with null values when calling `get_feature_flag_payload`.
+
+## 3.7.3 - 2024-11-25
+
+1. Use personless mode when sending an exception without a provided `distinct_id`.
+
+## 3.7.2 - 2024-11-19
+
+1. Add `type` property to exception stacks.
+
+## 3.7.1 - 2024-10-24
+
+1. Add `platform` property to each frame of exception stacks.
+
 ## 3.7.0 - 2024-10-03
 
 1. Adds a new `super_properties` parameter on the client that are appended to every /capture call.

CODEOWNERS

@@ -1 +1 @@
-@PostHog/team-feature-success
+@PostHog/team-feature-flags

example.py

@@ -1,7 +1,7 @@
 # PostHog Python library example
 
 # Import the library
-import time
+# import time
 
 import posthog
 

posthog/__init__.py

@@ -2,7 +2,7 @@ import datetime  # noqa: F401
 from typing import Callable, Dict, List, Optional, Tuple  # noqa: F401
 
 from posthog.client import Client
-from posthog.exception_capture import DEFAULT_DISTINCT_ID, Integrations  # noqa: F401
+from posthog.exception_capture import Integrations  # noqa: F401
 from posthog.version import VERSION
 
 __version__ = VERSION
@@ -289,7 +289,7 @@ def capture_exception(
     return _proxy(
         "capture_exception",
         exception=exception,
-        distinct_id=distinct_id or DEFAULT_DISTINCT_ID,
+        distinct_id=distinct_id,
         properties=properties,
         context=context,
         timestamp=timestamp,

posthog/client.py

@@ -4,13 +4,13 @@ import numbers
 import os
 import sys
 from datetime import datetime, timedelta
-from uuid import UUID
+from uuid import UUID, uuid4
 
 from dateutil.tz import tzutc
 from six import string_types
 
 from posthog.consumer import Consumer
-from posthog.exception_capture import DEFAULT_DISTINCT_ID, ExceptionCapture
+from posthog.exception_capture import ExceptionCapture
 from posthog.exception_utils import exc_info_from_error, exceptions_from_error_tuple, handle_in_app
 from posthog.feature_flags import InconclusiveMatchError, match_feature_flag_properties
 from posthog.poller import Poller
@@ -173,6 +173,15 @@ class Client(object):
         resp_data = self.get_decide(distinct_id, groups, person_properties, group_properties, disable_geoip)
         return resp_data["featureFlagPayloads"]
 
+    def get_feature_flags_and_payloads(
+        self, distinct_id, groups=None, person_properties=None, group_properties=None, disable_geoip=None
+    ):
+        resp_data = self.get_decide(distinct_id, groups, person_properties, group_properties, disable_geoip)
+        return {
+            "featureFlags": resp_data["featureFlags"],
+            "featureFlagPayloads": resp_data["featureFlagPayloads"],
+        }
+
     def get_decide(self, distinct_id, groups=None, person_properties=None, group_properties=None, disable_geoip=None):
         require("distinct_id", distinct_id, ID_TYPES)
 
@@ -295,6 +304,7 @@ class Client(object):
         timestamp=None,
         uuid=None,
         disable_geoip=None,
+        distinct_id=None,
     ):
         properties = properties or {}
         context = context or {}
@@ -302,6 +312,11 @@ class Client(object):
         require("group_key", group_key, ID_TYPES)
         require("properties", properties, dict)
 
+        if distinct_id:
+            require("distinct_id", distinct_id, ID_TYPES)
+        else:
+            distinct_id = "${}_{}".format(group_type, group_key)
+
         msg = {
             "event": "$groupidentify",
             "properties": {
@@ -309,7 +324,7 @@ class Client(object):
                 "$group_key": group_key,
                 "$group_set": properties,
             },
-            "distinct_id": "${}_{}".format(group_type, group_key),
+            "distinct_id": distinct_id,
             "timestamp": timestamp,
             "context": context,
             "uuid": uuid,
@@ -362,7 +377,7 @@ class Client(object):
     def capture_exception(
         self,
         exception=None,
-        distinct_id=DEFAULT_DISTINCT_ID,
+        distinct_id=None,
         properties=None,
         context=None,
         timestamp=None,
@@ -373,6 +388,13 @@ class Client(object):
         # this is important to ensure we don't unexpectedly re-raise exceptions in the user's code.
         try:
             properties = properties or {}
+
+            # if there's no distinct_id, we'll generate one and set personless mode
+            # via $process_person_profile = false
+            if distinct_id is None:
+                properties["$process_person_profile"] = False
+                distinct_id = uuid4()
+
             require("distinct_id", distinct_id, ID_TYPES)
             require("properties", properties, dict)
 
@@ -385,7 +407,7 @@ class Client(object):
                 self.log.warning("No exception information available")
                 return
 
-            # Format stack trace like sentry
+            # Format stack trace for cymbal
             all_exceptions_with_trace = exceptions_from_error_tuple(exc_info)
 
             # Add in-app property to frames in the exceptions
@@ -739,23 +761,52 @@ class Client(object):
                 groups=groups,
                 person_properties=person_properties,
                 group_properties=group_properties,
-                send_feature_flag_events=send_feature_flag_events,
-                only_evaluate_locally=True,
+                send_feature_flag_events=False,
+                # Disable automatic sending of feature flag events because we're manually handling event dispatch.
+                # This prevents sending events with empty data when `get_feature_flag` cannot be evaluated locally.
+                only_evaluate_locally=True,  # Enable local evaluation of feature flags to avoid making multiple requests to `/decide`.
                 disable_geoip=disable_geoip,
             )
 
         response = None
+        payload = None
 
         if match_value is not None:
-            response = self._compute_payload_locally(key, match_value)
+            payload = self._compute_payload_locally(key, match_value)
 
-        if response is None and not only_evaluate_locally:
-            decide_payloads = self.get_feature_payloads(
-                distinct_id, groups, person_properties, group_properties, disable_geoip
+        flag_was_locally_evaluated = payload is not None
+        if not flag_was_locally_evaluated and not only_evaluate_locally:
+            try:
+                responses_and_payloads = self.get_feature_flags_and_payloads(
+                    distinct_id, groups, person_properties, group_properties, disable_geoip
+                )
+                response = responses_and_payloads["featureFlags"].get(key, None)
+                payload = responses_and_payloads["featureFlagPayloads"].get(str(key).lower(), None)
+            except Exception as e:
+                self.log.exception(f"[FEATURE FLAGS] Unable to get feature flags and payloads: {e}")
+
+        feature_flag_reported_key = f"{key}_{str(response)}"
+
+        if (
+            feature_flag_reported_key not in self.distinct_ids_feature_flags_reported[distinct_id]
+            and send_feature_flag_events  # noqa: W503
+        ):
+            self.capture(
+                distinct_id,
+                "$feature_flag_called",
+                {
+                    "$feature_flag": key,
+                    "$feature_flag_response": response,
+                    "$feature_flag_payload": payload,
+                    "locally_evaluated": flag_was_locally_evaluated,
+                    f"$feature/{key}": response,
+                },
+                groups=groups,
+                disable_geoip=disable_geoip,
             )
-            response = decide_payloads.get(str(key).lower(), None)
+            self.distinct_ids_feature_flags_reported[distinct_id].add(feature_flag_reported_key)
 
-        return response
+        return payload
 
     def _compute_payload_locally(self, key, match_value):
         payload = None

posthog/exception_capture.py

@@ -12,9 +12,6 @@ class Integrations(str, Enum):
     Django = "django"
 
 
-DEFAULT_DISTINCT_ID = "python-exceptions"
-
-
 class ExceptionCapture:
     # TODO: Add client side rate limiting to prevent spamming the server with exceptions
 
@@ -61,14 +58,7 @@ class ExceptionCapture:
 
     def capture_exception(self, exception, metadata=None):
         try:
-            # if hasattr(sys, "ps1"):
-            #     # Disable the excepthook for interactive Python shells
-            #     return
-
-            distinct_id = metadata.get("distinct_id") if metadata else DEFAULT_DISTINCT_ID
-            # Make sure we have a distinct_id if its empty in metadata
-            distinct_id = distinct_id or DEFAULT_DISTINCT_ID
-
+            distinct_id = metadata.get("distinct_id") if metadata else None
             self.client.capture_exception(exception, distinct_id)
         except Exception as e:
             self.log.exception(f"Failed to capture exception: {e}")

posthog/exception_utils.py

@@ -369,6 +369,7 @@ def serialize_frame(
         tb_lineno = frame.f_lineno
 
     rv = {
+        "platform": "python",
         "filename": filename_for_module(module, abs_path) or None,
         "abs_path": os.path.abspath(abs_path) if abs_path else None,
         "function": function or "<unknown>",
@@ -417,7 +418,7 @@ def current_stacktrace(
 
     frames.reverse()
 
-    return {"frames": frames}
+    return {"frames": frames, "type": "raw"}
 
 
 def get_errno(exc_value):
@@ -503,7 +504,7 @@ def single_exception_from_error_tuple(
     ]
 
     if frames:
-        exception_value["stacktrace"] = {"frames": frames}
+        exception_value["stacktrace"] = {"frames": frames, "type": "raw"}
 
     return exception_value
 

posthog/test/test_client.py

@@ -104,11 +104,11 @@ class TestClient(unittest.TestCase):
         with mock.patch.object(Client, "capture", return_value=None) as patch_capture:
             client = self.client
             exception = Exception("test exception")
-            client.capture_exception(exception)
+            client.capture_exception(exception, distinct_id="distinct_id")
 
             self.assertTrue(patch_capture.called)
             capture_call = patch_capture.call_args[0]
-            self.assertEqual(capture_call[0], "python-exceptions")
+            self.assertEqual(capture_call[0], "distinct_id")
             self.assertEqual(capture_call[1], "$exception")
             self.assertEqual(
                 capture_call[2],
@@ -123,7 +123,7 @@ class TestClient(unittest.TestCase):
                             "value": "test exception",
                         }
                     ],
-                    "$exception_personURL": "https://us.i.posthog.com/project/random_key/person/python-exceptions",
+                    "$exception_personURL": "https://us.i.posthog.com/project/random_key/person/distinct_id",
                 },
             )
 
@@ -218,11 +218,11 @@ class TestClient(unittest.TestCase):
             try:
                 raise Exception("test exception")
             except Exception:
-                client.capture_exception()
+                client.capture_exception(distinct_id="distinct_id")
 
             self.assertTrue(patch_capture.called)
             capture_call = patch_capture.call_args[0]
-            self.assertEqual(capture_call[0], "python-exceptions")
+            self.assertEqual(capture_call[0], "distinct_id")
             self.assertEqual(capture_call[1], "$exception")
             self.assertEqual(capture_call[2]["$exception_type"], "Exception")
             self.assertEqual(capture_call[2]["$exception_message"], "test exception")
@@ -231,6 +231,10 @@ class TestClient(unittest.TestCase):
             self.assertEqual(capture_call[2]["$exception_list"][0]["module"], None)
             self.assertEqual(capture_call[2]["$exception_list"][0]["type"], "Exception")
             self.assertEqual(capture_call[2]["$exception_list"][0]["value"], "test exception")
+            self.assertEqual(
+                capture_call[2]["$exception_list"][0]["stacktrace"]["type"],
+                "raw",
+            )
             self.assertEqual(
                 capture_call[2]["$exception_list"][0]["stacktrace"]["frames"][0]["filename"],
                 "posthog/test/test_client.py",
@@ -711,6 +715,25 @@ class TestClient(unittest.TestCase):
         self.assertTrue(isinstance(msg["timestamp"], str))
         self.assertIsNone(msg.get("uuid"))
 
+    def test_basic_group_identify_with_distinct_id(self):
+        success, msg = self.client.group_identify("organization", "id:5", distinct_id="distinct_id")
+        self.assertTrue(success)
+        self.assertEqual(msg["event"], "$groupidentify")
+        self.assertEqual(msg["distinct_id"], "distinct_id")
+        self.assertEqual(
+            msg["properties"],
+            {
+                "$group_type": "organization",
+                "$group_key": "id:5",
+                "$group_set": {},
+                "$lib": "posthog-python",
+                "$lib_version": VERSION,
+                "$geoip_disable": True,
+            },
+        )
+        self.assertTrue(isinstance(msg["timestamp"], str))
+        self.assertIsNone(msg.get("uuid"))
+
     def test_advanced_group_identify(self):
         success, msg = self.client.group_identify(
             "organization", "id:5", {"trait": "value"}, {"ip": "192.168.0.1"}, datetime(2014, 9, 3), "new-uuid"
@@ -733,6 +756,35 @@ class TestClient(unittest.TestCase):
         self.assertEqual(msg["timestamp"], "2014-09-03T00:00:00+00:00")
         self.assertEqual(msg["context"]["ip"], "192.168.0.1")
 
+    def test_advanced_group_identify_with_distinct_id(self):
+        success, msg = self.client.group_identify(
+            "organization",
+            "id:5",
+            {"trait": "value"},
+            {"ip": "192.168.0.1"},
+            datetime(2014, 9, 3),
+            "new-uuid",
+            distinct_id="distinct_id",
+        )
+
+        self.assertTrue(success)
+        self.assertEqual(msg["event"], "$groupidentify")
+        self.assertEqual(msg["distinct_id"], "distinct_id")
+
+        self.assertEqual(
+            msg["properties"],
+            {
+                "$group_type": "organization",
+                "$group_key": "id:5",
+                "$group_set": {"trait": "value"},
+                "$lib": "posthog-python",
+                "$lib_version": VERSION,
+                "$geoip_disable": True,
+            },
+        )
+        self.assertEqual(msg["timestamp"], "2014-09-03T00:00:00+00:00")
+        self.assertEqual(msg["context"]["ip"], "192.168.0.1")
+
     def test_basic_alias(self):
         client = self.client
         success, msg = client.alias("previousId", "distinct_id")

posthog/test/test_exception_capture.py

@@ -29,7 +29,7 @@ def test_excepthook(tmpdir):
     assert b"LOL" in output
     assert b"DEBUG:posthog:data uploaded successfully" in output
     assert (
-        b'"$exception_list": [{"mechanism": {"type": "generic", "handled": true}, "module": null, "type": "ZeroDivisionError", "value": "division by zero", "stacktrace": {"frames": [{"filename": "app.py", "abs_path"'
+        b'"$exception_list": [{"mechanism": {"type": "generic", "handled": true}, "module": null, "type": "ZeroDivisionError", "value": "division by zero", "stacktrace": {"frames": [{"platform": "python", "filename": "app.py", "abs_path"'
         in output
     )
 
@@ -58,6 +58,6 @@ def test_trying_to_use_django_integration(tmpdir):
     assert b"LOL" in output
     assert b"DEBUG:posthog:data uploaded successfully" in output
     assert (
-        b'"$exception_list": [{"mechanism": {"type": "generic", "handled": true}, "module": null, "type": "ZeroDivisionError", "value": "division by zero", "stacktrace": {"frames": [{"filename": "app.py", "abs_path"'
+        b'"$exception_list": [{"mechanism": {"type": "generic", "handled": true}, "module": null, "type": "ZeroDivisionError", "value": "division by zero", "stacktrace": {"frames": [{"platform": "python", "filename": "app.py", "abs_path"'
         in output
     )

posthog/test/test_feature_flags.py

@@ -1632,9 +1632,10 @@ class TestLocalEvaluation(unittest.TestCase):
         )
         self.assertEqual(patch_decide.call_count, 0)
 
+    @mock.patch.object(Client, "capture")
     @mock.patch("posthog.client.decide")
-    def test_boolean_feature_flag_payload_decide(self, patch_decide):
-        patch_decide.return_value = {"featureFlagPayloads": {"person-flag": 300}}
+    def test_boolean_feature_flag_payload_decide(self, patch_decide, patch_capture):
+        patch_decide.return_value = {"featureFlags": {"person-flag": True}, "featureFlagPayloads": {"person-flag": 300}}
         self.assertEqual(
             self.client.get_feature_flag_payload(
                 "person-flag", "some-distinct-id", person_properties={"region": "USA"}
@@ -1649,6 +1650,8 @@ class TestLocalEvaluation(unittest.TestCase):
             300,
         )
         self.assertEqual(patch_decide.call_count, 2)
+        self.assertEqual(patch_capture.call_count, 1)
+        patch_capture.reset_mock()
 
     @mock.patch("posthog.client.decide")
     def test_multivariate_feature_flag_payloads(self, patch_decide):
@@ -2334,6 +2337,88 @@ class TestCaptureCalls(unittest.TestCase):
             disable_geoip=None,
         )
 
+    @mock.patch.object(Client, "capture")
+    @mock.patch("posthog.client.decide")
+    def test_capture_is_called_in_get_feature_flag_payload(self, patch_decide, patch_capture):
+        patch_decide.return_value = {
+            "featureFlags": {"person-flag": True},
+            "featureFlagPayloads": {"person-flag": 300},
+        }
+        client = Client(api_key=FAKE_TEST_API_KEY, personal_api_key=FAKE_TEST_API_KEY)
+
+        client.feature_flags = [
+            {
+                "id": 1,
+                "name": "Beta Feature",
+                "key": "person-flag",
+                "is_simple_flag": False,
+                "active": True,
+                "filters": {
+                    "groups": [
+                        {
+                            "properties": [{"key": "region", "value": "USA"}],
+                            "rollout_percentage": 100,
+                        }
+                    ],
+                },
+            }
+        ]
+
+        # Call get_feature_flag_payload with match_value=None to trigger get_feature_flag
+        client.get_feature_flag_payload(
+            key="person-flag", distinct_id="some-distinct-id", person_properties={"region": "USA", "name": "Aloha"}
+        )
+
+        # Assert that capture was called once, with the correct parameters
+        self.assertEqual(patch_capture.call_count, 1)
+        patch_capture.assert_called_with(
+            "some-distinct-id",
+            "$feature_flag_called",
+            {
+                "$feature_flag": "person-flag",
+                "$feature_flag_response": True,
+                "$feature_flag_payload": 300,
+                "locally_evaluated": False,
+                "$feature/person-flag": True,
+            },
+            groups={},
+            disable_geoip=None,
+        )
+
+        # Reset mocks for further tests
+        patch_capture.reset_mock()
+        patch_decide.reset_mock()
+
+        # Call get_feature_flag_payload again for the same user; capture should not be called again because we've already reported an event for this distinct_id + flag
+        client.get_feature_flag_payload(
+            key="person-flag", distinct_id="some-distinct-id", person_properties={"region": "USA", "name": "Aloha"}
+        )
+
+        self.assertEqual(patch_capture.call_count, 0)
+        patch_capture.reset_mock()
+
+        # Call get_feature_flag_payload for a different user; capture should be called
+        client.get_feature_flag_payload(
+            key="person-flag", distinct_id="some-distinct-id2", person_properties={"region": "USA", "name": "Aloha"}
+        )
+
+        self.assertEqual(patch_capture.call_count, 1)
+        patch_capture.assert_called_with(
+            "some-distinct-id2",
+            "$feature_flag_called",
+            {
+                "$feature_flag": "person-flag",
+                "$feature_flag_response": True,
+                "$feature_flag_payload": 300,
+                "locally_evaluated": False,
+                "$feature/person-flag": True,
+            },
+            groups={},
+            disable_geoip=None,
+        )
+
+        patch_capture.reset_mock()
+
     @mock.patch.object(Client, "capture")
     @mock.patch("posthog.client.decide")
     def test_disable_geoip_get_flag_capture_call(self, patch_decide, patch_capture):

posthog/version.py

@@ -1,4 +1,4 @@
-VERSION = "3.7.0"
+VERSION = "3.7.5"
 
 if __name__ == "__main__":
     print(VERSION, end="")  # noqa: T201