feat: emit $is_server property on captured events by turnipdabeets · Pull Request #643 · PostHog/posthog-python

turnipdabeets · 2026-06-02T19:37:24Z

Adds $is_server: true to every event captured by this SDK so PostHog ingestion can identify server-side events.

greptile-apps · 2026-06-02T19:40:38Z

Comments Outside Diff (1)

posthog/client.py, line 1319-1330 (link)

$is_server is set before the super_properties merge, so a caller who happens to include "$is_server" in their super_properties dict will silently override the SDK-set value. Since this property exists specifically to guarantee every event from this SDK is tagged as a server-side event, it should be set after the merge so it is never overridden. ($lib and $lib_version share the same gap, but $is_server is the property the ingestion pipeline relies on for this classification.)

Prompt To Fix With AI

This is a comment left during a code review.
Path: posthog/client.py
Line: 1319-1330

Comment:
`$is_server` is set before the `super_properties` merge, so a caller who happens to include `"$is_server"` in their `super_properties` dict will silently override the SDK-set value. Since this property exists specifically to guarantee every event from this SDK is tagged as a server-side event, it should be set after the merge so it is never overridden. (`$lib` and `$lib_version` share the same gap, but `$is_server` is the property the ingestion pipeline relies on for this classification.)



How can I resolve this? If you propose a fix, please make it concise.

Prompt To Fix All With AI

Fix the following 2 code review issues. Work through them one at a time, proposing concise fixes.

---

### Issue 1 of 2
posthog/client.py:1319-1330
`$is_server` is set before the `super_properties` merge, so a caller who happens to include `"$is_server"` in their `super_properties` dict will silently override the SDK-set value. Since this property exists specifically to guarantee every event from this SDK is tagged as a server-side event, it should be set after the merge so it is never overridden. (`$lib` and `$lib_version` share the same gap, but `$is_server` is the property the ingestion pipeline relies on for this classification.)

```suggestion
        msg["properties"]["$lib"] = "posthog-python"
        msg["properties"]["$lib_version"] = VERSION

        if disable_geoip is None:
            disable_geoip = self.disable_geoip

        if disable_geoip:
            msg["properties"]["$geoip_disable"] = True

        if self.super_properties:
            msg["properties"] = {**msg["properties"], **self.super_properties}

        msg["properties"]["$is_server"] = True
```

### Issue 2 of 2
posthog/test/test_client.py:162
**Missing test for super_properties override**

The test only verifies that `$is_server` is present in the happy-path `test_basic_capture`. Given the placement of the property before the `super_properties` merge, a test that constructs a client with `super_properties={"$is_server": False}` and asserts the outgoing event still has `$is_server == True` would catch the override issue and anchor the intended invariant.

_{Reviews (1): Last reviewed commit: "feat: emit $is_server property on captur..." | Re-trigger Greptile}

greptile-apps · 2026-06-02T19:40:43Z

            self.assertEqual(msg["distinct_id"], "distinct_id")
            self.assertEqual(msg["properties"]["$lib"], "posthog-python")
            self.assertEqual(msg["properties"]["$lib_version"], VERSION)
+            self.assertEqual(msg["properties"]["$is_server"], True)


Missing test for super_properties override

The test only verifies that $is_server is present in the happy-path test_basic_capture. Given the placement of the property before the super_properties merge, a test that constructs a client with super_properties={"$is_server": False} and asserts the outgoing event still has $is_server == True would catch the override issue and anchor the intended invariant.

Prompt To Fix With AI

This is a comment left during a code review. Path: posthog/test/test_client.py Line: 162 Comment: **Missing test for super_properties override** The test only verifies that `$is_server` is present in the happy-path `test_basic_capture`. Given the placement of the property before the `super_properties` merge, a test that constructs a client with `super_properties={"$is_server": False}` and asserts the outgoing event still has `$is_server == True` would catch the override issue and anchor the intended invariant. How can I resolve this? If you propose a fix, please make it concise.

Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!

Good catch — addressed both. $is_server is now set after the super_properties merge so a user-provided super property can no longer override the SDK classification, and added test_is_server_not_overridden_by_super_properties asserting it stays True even when super_properties sets $is_server: False.

github-actions · 2026-06-02T19:41:19Z

posthog-python Compliance Report

Date: 2026-06-02 23:17:52 UTC
Duration: 176119ms

✅ All Tests Passed!

45/45 tests passed

Capture Tests

✅ 29/29 tests passed

View Details

Test	Status	Duration
Format Validation.Event Has Required Fields	✅	516ms
Format Validation.Event Has Uuid	✅	1508ms
Format Validation.Event Has Lib Properties	✅	1507ms
Format Validation.Distinct Id Is String	✅	1507ms
Format Validation.Token Is Present	✅	1507ms
Format Validation.Custom Properties Preserved	✅	1507ms
Format Validation.Event Has Timestamp	✅	1507ms
Retry Behavior.Retries On 503	✅	9515ms
Retry Behavior.Does Not Retry On 400	✅	3509ms
Retry Behavior.Does Not Retry On 401	✅	3507ms
Retry Behavior.Respects Retry After Header	✅	9511ms
Retry Behavior.Implements Backoff	✅	23522ms
Retry Behavior.Retries On 500	✅	7512ms
Retry Behavior.Retries On 502	✅	7514ms
Retry Behavior.Retries On 504	✅	7510ms
Retry Behavior.Max Retries Respected	✅	23534ms
Deduplication.Generates Unique Uuids	✅	1497ms
Deduplication.Preserves Uuid On Retry	✅	7515ms
Deduplication.Preserves Uuid And Timestamp On Retry	✅	14521ms
Deduplication.Preserves Uuid And Timestamp On Batch Retry	✅	7507ms
Deduplication.No Duplicate Events In Batch	✅	1505ms
Deduplication.Different Events Have Different Uuids	✅	1507ms
Compression.Sends Gzip When Enabled	✅	1507ms
Batch Format.Uses Proper Batch Structure	✅	1507ms
Batch Format.Flush With No Events Sends Nothing	✅	1005ms
Batch Format.Multiple Events Batched Together	✅	1505ms
Error Handling.Does Not Retry On 403	✅	3507ms
Error Handling.Does Not Retry On 413	✅	3508ms
Error Handling.Retries On 408	✅	7516ms

Feature_Flags Tests

✅ 16/16 tests passed

View Details

Test	Status	Duration
Request Payload.Request With Person Properties Device Id	✅	1003ms
Request Payload.Flags Request Uses V2 Query Param	✅	1006ms
Request Payload.Flags Request Hits Flags Path Not Decide	✅	1007ms
Request Payload.Flags Request Omits Authorization Header	✅	1007ms
Request Payload.Token In Flags Body Matches Init	✅	1007ms
Request Payload.Groups Round Trip	✅	1007ms
Request Payload.Groups Default To Empty Object	✅	1006ms
Request Payload.Person Properties Distinct Id Auto Populated When Caller Omits It	✅	1007ms
Request Payload.Disable Geoip False Propagates As Geoip Disable False	✅	1007ms
Request Payload.Disable Geoip Omitted Defaults To False	✅	1007ms
Request Payload.Flag Keys To Evaluate Contains Only Requested Key	✅	1006ms
Request Lifecycle.No Flags Request On Init Alone	✅	502ms
Request Lifecycle.No Flags Request On Normal Capture	✅	1508ms
Request Lifecycle.Two Flag Calls Produce Two Remote Requests	✅	1012ms
Request Lifecycle.Mock Response Value Is Returned To Caller	✅	1002ms
Side Effect Events.Get Feature Flag Captures Feature Flag Called Event	✅	1509ms

…ridden

ioannisj

Approving to unblock, left a couple of comments though

ioannisj · 2026-06-03T06:32:26Z

@@ -0,0 +1,5 @@
+---
+pypi/posthog: patch


This should probably be minor since we are adding a new constructor param?

Your call here but if we change we'll need to update all other SDKs that provide a config option as well

ioannisj · 2026-06-03T06:34:20Z

+        if self.is_server:
+            msg["properties"]["$is_server"] = True


Curious, why do we omit vs set to False?

makes sense to me, we dont send on the client side so its the same semantics

ioannisj · 2026-06-03T06:35:52Z

 project_api_key = None  # type: Optional[str]
 poll_interval = 30  # type: int
 disable_geoip = True  # type: bool
+is_server = True  # type: bool


We are missing a docstring entry above

ioannisj · 2026-06-03T07:05:01Z

 project_api_key = None  # type: Optional[str]
 poll_interval = 30  # type: int
 disable_geoip = True  # type: bool
+is_server = True  # type: bool


Bool would be hard to change in the future without breaking changes if needed. Wondering if we should be defining as String (server, client for now) which will give us flexibility for the future. Based on the comment here for example this could also be "cli" (we'd need to rename the property of course to maybe something like $runtime_type)

I'll hold back review on other SDKs since they follow the same patters so these points can be discussed here?

CLI is also a client side, its all about if its running in the users machine or not at the end of the day, does not matter if its a desktop app, a mobile app, a CLI, etc
its either local or cloud, not sure if we need more than a boolean but good point

feat: emit $is_server property on captured events

cd1db41

turnipdabeets requested a review from a team as a code owner June 2, 2026 19:37

greptile-apps Bot reviewed Jun 2, 2026

View reviewed changes

turnipdabeets added 4 commits June 2, 2026 15:49

chore: add changeset for $is_server property

c2be91e

feat: make $is_server configurable (default true)

2f1a29d

fix: set $is_server after super_properties merge so it cannot be over…

214b79f

…ridden

test: include $is_server in group_identify exact-match assertions

2037d02

ioannisj approved these changes Jun 3, 2026

View reviewed changes

ioannisj reviewed Jun 3, 2026

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: emit $is_server property on captured events#643

feat: emit $is_server property on captured events#643
turnipdabeets wants to merge 5 commits into
mainfrom
feat/add-is-server-property

turnipdabeets commented Jun 2, 2026

Uh oh!

greptile-apps Bot commented Jun 2, 2026 •

edited

Loading

Comments Outside Diff (1)

Uh oh!

greptile-apps Bot Jun 2, 2026

Uh oh!

turnipdabeets Jun 2, 2026

Uh oh!

github-actions Bot commented Jun 2, 2026 •

edited

Loading

Uh oh!

ioannisj left a comment

Uh oh!

ioannisj Jun 3, 2026

Uh oh!

ioannisj Jun 3, 2026

Uh oh!

ioannisj Jun 3, 2026

Uh oh!

marandaneto Jun 3, 2026

Uh oh!

ioannisj Jun 3, 2026

Uh oh!

ioannisj Jun 3, 2026

Uh oh!

marandaneto Jun 3, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Conversation

turnipdabeets commented Jun 2, 2026

Uh oh!

greptile-apps Bot commented Jun 2, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Comments Outside Diff (1)

Uh oh!

greptile-apps Bot Jun 2, 2026

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

github-actions Bot commented Jun 2, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

posthog-python Compliance Report

✅ All Tests Passed!

Capture Tests

Feature_Flags Tests

Uh oh!

ioannisj left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

greptile-apps Bot commented Jun 2, 2026 •

edited

Loading

github-actions Bot commented Jun 2, 2026 •

edited

Loading